[PATCH v2 0/3] meson-gx: Add mali-450 support

[PATCH v2 0/3] meson-gx: Add mali-450 support

[Neil Armstrong]

Since the merge of the Mali dt bindings at [1], add support for Mali clocks
and DT node.

The Mali is clocked by two identical clock paths behind a glitch free mux
to safely change frequency while running.
So these clocks must be added to the meson-gxbb clock controller.

Changes since v1 at [2] :
 - Remove GP0 fixes, this will pushed later, for base frequency it can depend on fclk_div3
 - Add GXL support
 - rebase on clk-next and jbrunet's patchset at [3] and [4]
 - get rid of composite clocks, this adds more clocks IDs and exposes 2 more clocks

[4] http://lkml.kernel.org/r/20170228093016.5624-1-jbrunet@baylibre.com
[3] http://lkml.kernel.org/r/20170228133002.17894-1-jbrunet@baylibre.com
[2] http://lkml.kernel.org/r/1486721084-1383-1-git-send-email-narmstrong@baylibre.com
[1] http://lkml.kernel.org/r/b098c4fa9fce88361cca20417978734d0e1b5cca.1485939041.git-series.maxime.ripard@free-electrons.com

Neil Armstrong (3):
  clk: meson-gxbb: Add MALI clock IDS
  clk: meson-gxbb: Add MALI clocks
  ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi      |  37 ++++++
 arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi  |  43 +++++++
 arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi |   1 +
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi |   1 +
 drivers/clk/meson/gxbb.c                         | 139 +++++++++++++++++++++++
 drivers/clk/meson/gxbb.h                         |   9 +-
 include/dt-bindings/clock/gxbb-clkc.h            |   5 +
 7 files changed, 234 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi

-- 
1.9.1

[PATCH v2 1/3] clk: meson-gxbb: Add MALI clock IDS

[Neil Armstrong]

Add missing MALI clock IDs and expose the muxes and gates in the dt-bindings.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/clk/meson/gxbb.h              | 9 ++++++++-
 include/dt-bindings/clock/gxbb-clkc.h | 5 +++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/meson/gxbb.h b/drivers/clk/meson/gxbb.h
index 95a83b7..2ec8f8a 100644
--- a/drivers/clk/meson/gxbb.h
+++ b/drivers/clk/meson/gxbb.h
@@ -268,8 +268,15 @@
 /* CLKID_SAR_ADC_CLK */
 /* CLKID_SAR_ADC_SEL */
 #define CLKID_SAR_ADC_DIV	  99
+/* CLKID_MALI_0_SEL */
+#define CLKID_MALI_0_DIV	 101
+/* CLKID_MALI_0	*/
+/* CLKID_MALI_1_SEL */
+#define CLKID_MALI_1_DIV	 104
+/* CLKID_MALI_1	*/
+/* CLKID_MALI	*/
 
-#define NR_CLKS			  100
+#define NR_CLKS			  107
 
 /* include the CLKIDs that have been made part of the stable DT binding */
 #include <dt-bindings/clock/gxbb-clkc.h>
diff --git a/include/dt-bindings/clock/gxbb-clkc.h b/include/dt-bindings/clock/gxbb-clkc.h
index f08f06d..ef7d6b7 100644
--- a/include/dt-bindings/clock/gxbb-clkc.h
+++ b/include/dt-bindings/clock/gxbb-clkc.h
@@ -35,5 +35,10 @@
 #define CLKID_SD_EMMC_C		96
 #define CLKID_SAR_ADC_CLK	97
 #define CLKID_SAR_ADC_SEL	98
+#define CLKID_MALI_0_SEL	100
+#define CLKID_MALI_0		102
+#define CLKID_MALI_1_SEL	103
+#define CLKID_MALI_1		105
+#define CLKID_MALI		106
 
 #endif /* __GXBB_CLKC_H */
-- 
1.9.1

[PATCH v2 2/3] clk: meson-gxbb: Add MALI clocks

[Neil Armstrong]

The Mali is clocked by two identical clock paths behind a glitch free mux
to safely change frequency while running.

The two "mali_0" and "mali_1" clocks are composed of a mux, divider and gate.
Expose these two clocks trees using generic clocks.
Finally the glitch free mux is added as "mali" clock.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/clk/meson/gxbb.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index a52063f..31f6090 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -634,6 +634,131 @@
 	},
 };
 
+/*
+ * The MALI IP is clocked by two identical clocks (mali_0 and mali_1)
+ * muxed by a glitch-free switch.
+ */
+
+static u32 mux_table_mali_0_1[] = {0, 1, 2, 3, 4, 5, 6, 7};
+const char *gxbb_mali_0_1_parent_names[] = {
+	"xtal", "gp0_pll", "mpll2", "mpll1", "fclk_div7",
+	"fclk_div4", "fclk_div3", "fclk_div5"
+};
+
+static struct clk_mux gxbb_mali_0_sel = {
+	.reg = (void *)HHI_MALI_CLK_CNTL,
+	.mask = 0x7,
+	.shift = 9,
+	.table = mux_table_mali_0_1,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "mali_0_sel",
+		.ops = &clk_mux_ops,
+		/*
+		 * bits 10:9 selects from 8 possible parents:
+		 * xtal, gp0_pll, mpll2, mpll1, fclk_div7,
+		 * fclk_div4, fclk_div3, fclk_div5
+		 */
+		.parent_names = gxbb_mali_0_1_parent_names,
+		.num_parents = 8,
+		.flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
+	},
+};
+
+static struct clk_divider gxbb_mali_0_div = {
+	.reg = (void *)HHI_MALI_CLK_CNTL,
+	.shift = 0,
+	.width = 7,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "mali_0_div",
+		.ops = &clk_divider_ops,
+		.parent_names = (const char *[]){ "mali_0_sel" },
+		.num_parents = 1,
+		.flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
+	},
+};
+
+static struct clk_gate gxbb_mali_0 = {
+	.reg = (void *)HHI_MALI_CLK_CNTL,
+	.bit_idx = 8,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "mali_0",
+		.ops = &clk_gate_ops,
+		.parent_names = (const char *[]){ "mali_0_div" },
+		.num_parents = 1,
+		.flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED),
+	},
+};
+
+static struct clk_mux gxbb_mali_1_sel = {
+	.reg = (void *)HHI_MALI_CLK_CNTL,
+	.mask = 0x7,
+	.shift = 25,
+	.table = mux_table_mali_0_1,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "mali_1_sel",
+		.ops = &clk_mux_ops,
+		/*
+		 * bits 10:9 selects from 8 possible parents:
+		 * xtal, gp0_pll, mpll2, mpll1, fclk_div7,
+		 * fclk_div4, fclk_div3, fclk_div5
+		 */
+		.parent_names = gxbb_mali_0_1_parent_names,
+		.num_parents = 8,
+		.flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
+	},
+};
+
+static struct clk_divider gxbb_mali_1_div = {
+	.reg = (void *)HHI_MALI_CLK_CNTL,
+	.shift = 16,
+	.width = 7,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "mali_1_div",
+		.ops = &clk_divider_ops,
+		.parent_names = (const char *[]){ "mali_1_sel" },
+		.num_parents = 1,
+		.flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
+	},
+};
+
+static struct clk_gate gxbb_mali_1 = {
+	.reg = (void *)HHI_MALI_CLK_CNTL,
+	.bit_idx = 24,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "mali_1",
+		.ops = &clk_gate_ops,
+		.parent_names = (const char *[]){ "mali_1_div" },
+		.num_parents = 1,
+		.flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED),
+	},
+};
+
+static u32 mux_table_mali[] = {0, 1};
+const char *gxbb_mali_parent_names[] = {
+	"mali_0", "mali_1"
+};
+
+static struct clk_mux gxbb_mali = {
+	.reg = (void *)HHI_MALI_CLK_CNTL,
+	.mask = 1,
+	.shift = 31,
+	.table = mux_table_mali,
+	.lock = &clk_lock,
+	.hw.init = &(struct clk_init_data){
+		.name = "mali",
+		.ops = &clk_mux_ops,
+		.parent_names = gxbb_mali_parent_names,
+		.num_parents = 2,
+		.flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
+	},
+};
+
 /* Everything Else (EE) domain gates */
 static MESON_GATE(gxbb_ddr, HHI_GCLK_MPEG0, 0);
 static MESON_GATE(gxbb_dos, HHI_GCLK_MPEG0, 1);
@@ -827,6 +952,13 @@
 		[CLKID_SAR_ADC_CLK]	    = &gxbb_sar_adc_clk.hw,
 		[CLKID_SAR_ADC_SEL]	    = &gxbb_sar_adc_clk_sel.hw,
 		[CLKID_SAR_ADC_DIV]	    = &gxbb_sar_adc_clk_div.hw,
+		[CLKID_MALI_0_SEL]	    = &gxbb_mali_0_sel.hw,
+		[CLKID_MALI_0_DIV]	    = &gxbb_mali_0_div.hw,
+		[CLKID_MALI_0]		    = &gxbb_mali_0.hw,
+		[CLKID_MALI_1_SEL]	    = &gxbb_mali_1_sel.hw,
+		[CLKID_MALI_1_DIV]	    = &gxbb_mali_1_div.hw,
+		[CLKID_MALI_1]		    = &gxbb_mali_1.hw,
+		[CLKID_MALI]		    = &gxbb_mali.hw,
 	},
 	.num = NR_CLKS,
 };
@@ -930,16 +1062,23 @@
 	&gxbb_emmc_b,
 	&gxbb_emmc_c,
 	&gxbb_sar_adc_clk,
+	&gxbb_mali_0,
+	&gxbb_mali_1,
 };
 
 static struct clk_mux *gxbb_clk_muxes[] = {
 	&gxbb_mpeg_clk_sel,
 	&gxbb_sar_adc_clk_sel,
+	&gxbb_mali_0_sel,
+	&gxbb_mali_1_sel,
+	&gxbb_mali,
 };
 
 static struct clk_divider *gxbb_clk_dividers[] = {
 	&gxbb_mpeg_clk_div,
 	&gxbb_sar_adc_clk_div,
+	&gxbb_mali_0_div,
+	&gxbb_mali_1_div,
 };
 
 static int gxbb_clkc_probe(struct platform_device *pdev)
-- 
1.9.1

[PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Neil Armstrong]

The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs.

The node is simply added in the meson-gxbb.dtsi file.

For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
dtsi files.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi      | 37 ++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi  | 43 ++++++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi |  1 +
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi |  1 +
 4 files changed, 82 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index b353073..4f7ae6a 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -474,6 +474,43 @@
 	};
 };
 
+&apb {
+	mali: gpu@c0000 {
+		compatible = "amlogic,meson-gxbb-mali", "arm,mali-450";
+		reg = <0x0 0xc0000 0x0 0x40000>;
+		interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "gp", "gpmmu", "pp", "pmu",
+			"pp0", "ppmmu0", "pp1", "ppmmu1",
+			"pp2", "ppmmu2";
+		clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_MALI>;
+		clock-names = "bus", "core";
+
+		/*
+		 * Mali clocking is provided by two identical clock paths
+		 * MALI_0 and MALI_1 muxed to a single clock by a glitch
+		 * free mux to safely change frequency while running.
+		 */
+		assigned-clocks = <&clkc CLKID_MALI_0_SEL>,
+				  <&clkc CLKID_MALI_0>,
+				  <&clkc CLKID_MALI>; /* Glitch free mux */
+		assigned-clock-parents = <&clkc CLKID_FCLK_DIV3>,
+					 <0>, /* Do Nothing */
+					 <&clkc CLKID_MALI_0>;
+		assigned-clock-rates = <0>, /* Do Nothing */
+				       <666666666>,
+				       <0>; /* Do Nothing */
+	};
+};
+
 &i2c_A {
 	clocks = <&clkc CLKID_I2C>;
 };
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi
new file mode 100644
index 0000000..f06cc234
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2017 BayLibre SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+&apb {
+	mali: gpu@c0000 {
+		compatible = "amlogic,meson-gxbb-mali", "arm,mali-450";
+		reg = <0x0 0xc0000 0x0 0x40000>;
+		interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "gp", "gpmmu", "pp", "pmu",
+			"pp0", "ppmmu0", "pp1", "ppmmu1",
+			"pp2", "ppmmu2";
+		clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_MALI>;
+		clock-names = "bus", "core";
+
+		/*
+		 * Mali clocking is provided by two identical clock paths
+		 * MALI_0 and MALI_1 muxed to a single clock by a glitch
+		 * free mux to safely change frequency while running.
+		 */
+		assigned-clocks = <&clkc CLKID_MALI_0_SEL>,
+				  <&clkc CLKID_MALI_0>,
+				  <&clkc CLKID_MALI>; /* Glitch free mux */
+		assigned-clock-parents = <&clkc CLKID_FCLK_DIV3>,
+					 <0>, /* Do Nothing */
+					 <&clkc CLKID_MALI_0>;
+		assigned-clock-rates = <0>, /* Do Nothing */
+				       <666666666>,
+				       <0>; /* Do Nothing */
+	};
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
index 615308e..5a90e30 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
@@ -42,6 +42,7 @@
  */
 
 #include "meson-gxl.dtsi"
+#include "meson-gxl-mali.dtsi"
 
 / {
 	compatible = "amlogic,s905d", "amlogic,meson-gxl";
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
index 08237ee..0f78d83 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
@@ -42,6 +42,7 @@
  */
 
 #include "meson-gxl.dtsi"
+#include "meson-gxl-mali.dtsi"
 
 / {
 	compatible = "amlogic,s905x", "amlogic,meson-gxl";
-- 
1.9.1

Re: [PATCH v2 2/3] clk: meson-gxbb: Add MALI clocks

[Stephen Boyd]

On 03/01, Neil Armstrong wrote:
> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
> index a52063f..31f6090 100644
> --- a/drivers/clk/meson/gxbb.c
> +++ b/drivers/clk/meson/gxbb.c
> @@ -634,6 +634,131 @@
>  	},
>  };
>  
> +/*
> + * The MALI IP is clocked by two identical clocks (mali_0 and mali_1)
> + * muxed by a glitch-free switch.
> + */
> +
> +static u32 mux_table_mali_0_1[] = {0, 1, 2, 3, 4, 5, 6, 7};
> +const char *gxbb_mali_0_1_parent_names[] = {

static?

> +	"xtal", "gp0_pll", "mpll2", "mpll1", "fclk_div7",
> +	"fclk_div4", "fclk_div3", "fclk_div5"
> +};
> +
[..]
> +	.reg = (void *)HHI_MALI_CLK_CNTL,
> +	.bit_idx = 24,
> +	.lock = &clk_lock,
> +	.hw.init = &(struct clk_init_data){
> +		.name = "mali_1",
> +		.ops = &clk_gate_ops,
> +		.parent_names = (const char *[]){ "mali_1_div" },
> +		.num_parents = 1,
> +		.flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED),
> +	},
> +};
> +
> +static u32 mux_table_mali[] = {0, 1};
> +const char *gxbb_mali_parent_names[] = {

static?

> +	"mali_0", "mali_1"
> +};
[...]
>  static struct clk_mux *gxbb_clk_muxes[] = {
>  	&gxbb_mpeg_clk_sel,
>  	&gxbb_sar_adc_clk_sel,
> +	&gxbb_mali_0_sel,
> +	&gxbb_mali_1_sel,
> +	&gxbb_mali,
>  };
>  
>  static struct clk_divider *gxbb_clk_dividers[] = {

Can these arrays be const? If so, please do that in a separate
patch.

>  	&gxbb_mpeg_clk_div,
>  	&gxbb_sar_adc_clk_div,
> +	&gxbb_mali_0_div,
> +	&gxbb_mali_1_div,
>  };
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 2/3] clk: meson-gxbb: Add MALI clocks

[Neil Armstrong]

Hi Stephen,

On 03/01/2017 08:11 PM, Stephen Boyd wrote:
> On 03/01, Neil Armstrong wrote:
>> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
>> index a52063f..31f6090 100644
>> --- a/drivers/clk/meson/gxbb.c
>> +++ b/drivers/clk/meson/gxbb.c
>> @@ -634,6 +634,131 @@
>>  	},
>>  };
>>  
>> +/*
>> + * The MALI IP is clocked by two identical clocks (mali_0 and mali_1)
>> + * muxed by a glitch-free switch.
>> + */
>> +
>> +static u32 mux_table_mali_0_1[] = {0, 1, 2, 3, 4, 5, 6, 7};
>> +const char *gxbb_mali_0_1_parent_names[] = {
> 
> static?

Will do !

> 
>> +	"xtal", "gp0_pll", "mpll2", "mpll1", "fclk_div7",
>> +	"fclk_div4", "fclk_div3", "fclk_div5"
>> +};
>> +
> [..]
>> +	.reg = (void *)HHI_MALI_CLK_CNTL,
>> +	.bit_idx = 24,
>> +	.lock = &clk_lock,
>> +	.hw.init = &(struct clk_init_data){
>> +		.name = "mali_1",
>> +		.ops = &clk_gate_ops,
>> +		.parent_names = (const char *[]){ "mali_1_div" },
>> +		.num_parents = 1,
>> +		.flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED),
>> +	},
>> +};
>> +
>> +static u32 mux_table_mali[] = {0, 1};
>> +const char *gxbb_mali_parent_names[] = {
> 
> static?
> 
>> +	"mali_0", "mali_1"
>> +};
> [...]
>>  static struct clk_mux *gxbb_clk_muxes[] = {
>>  	&gxbb_mpeg_clk_sel,
>>  	&gxbb_sar_adc_clk_sel,
>> +	&gxbb_mali_0_sel,
>> +	&gxbb_mali_1_sel,
>> +	&gxbb_mali,
>>  };
>>  
>>  static struct clk_divider *gxbb_clk_dividers[] = {
> 
> Can these arrays be const? If so, please do that in a separate
> patch.

Hmm, these were introduced by jerome, he should update them accordingly.

>>  	&gxbb_mpeg_clk_div,
>>  	&gxbb_sar_adc_clk_div,
>> +	&gxbb_mali_0_div,
>> +	&gxbb_mali_1_div,
>>  };


Thanks,
Neil

Re: [PATCH v2 2/3] clk: meson-gxbb: Add MALI clocks

[Jerome Brunet]

On Thu, 2017-03-02 at 12:07 +0100, Neil Armstrong wrote:
> Hi Stephen,
> 
> On 03/01/2017 08:11 PM, Stephen Boyd wrote:
> > On 03/01, Neil Armstrong wrote:
> > > diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
> > > index a52063f..31f6090 100644
> > > --- a/drivers/clk/meson/gxbb.c
> > > +++ b/drivers/clk/meson/gxbb.c
> > > @@ -634,6 +634,131 @@
> > >  	},
> > >  };
> > >  
> > > +/*
> > > + * The MALI IP is clocked by two identical clocks (mali_0 and
> > > mali_1)
> > > + * muxed by a glitch-free switch.
> > > + */
> > > +
> > > +static u32 mux_table_mali_0_1[] = {0, 1, 2, 3, 4, 5, 6, 7};
> > > +const char *gxbb_mali_0_1_parent_names[] = {
> > 
> > static?
> 
> Will do !
> 
> > 
> > > +	"xtal", "gp0_pll", "mpll2", "mpll1", "fclk_div7",
> > > +	"fclk_div4", "fclk_div3", "fclk_div5"
> > > +};
> > > +
> > 
> > [..]
> > > +	.reg = (void *)HHI_MALI_CLK_CNTL,
> > > +	.bit_idx = 24,
> > > +	.lock = &clk_lock,
> > > +	.hw.init = &(struct clk_init_data){
> > > +		.name = "mali_1",
> > > +		.ops = &clk_gate_ops,
> > > +		.parent_names = (const char *[]){ "mali_1_div"
> > > },
> > > +		.num_parents = 1,
> > > +		.flags = (CLK_SET_RATE_PARENT |
> > > CLK_IGNORE_UNUSED),
> > > +	},
> > > +};
> > > +
> > > +static u32 mux_table_mali[] = {0, 1};
> > > +const char *gxbb_mali_parent_names[] = {
> > 
> > static?
> > 
> > > +	"mali_0", "mali_1"
> > > +};
> > 
> > [...]
> > >  static struct clk_mux *gxbb_clk_muxes[] = {
> > >  	&gxbb_mpeg_clk_sel,
> > >  	&gxbb_sar_adc_clk_sel,
> > > +	&gxbb_mali_0_sel,
> > > +	&gxbb_mali_1_sel,
> > > +	&gxbb_mali,
> > >  };
> > >  
> > >  static struct clk_divider *gxbb_clk_dividers[] = {
> > 
> > Can these arrays be const? If so, please do that in a separate
> > patch.
> 
> Hmm, these were introduced by jerome, he should update them
> accordingly.
> 

Will do

> > >  	&gxbb_mpeg_clk_div,
> > >  	&gxbb_sar_adc_clk_div,
> > > +	&gxbb_mali_0_div,
> > > +	&gxbb_mali_1_div,
> > >  };
> 
> 
> Thanks,
> Neil

Re: [PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Andreas Färber]

Hi Neil,

Am 01.03.2017 um 11:46 schrieb Neil Armstrong:
> The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs.

First of all, any reason you're upper-casing Mali in the commit message?
ARM doesn't.

> 
> The node is simply added in the meson-gxbb.dtsi file.

The GXBB part looks fine on a quick look.

> 
> For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
> patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
> dtsi files.

This part is slightly confusing though.

What exactly is the GXL vs. GXM difference that this can't be handled by
overriding node properties compatible/interrupts/clocks? I am missing a
GXM patch in this series as rationale for doing it this way.

In particular I am wondering whether the whole GXM-inherits-from-GXL
concept is flawed and should be adjusted if this leads to secondary
.dtsi files like this: My proposal would be to instead create a
meson-gxl-gxm.dtsi, that meson-gxl.dtsi and meson-gxm.dtsi can inherit
the current common parts from, then the Mali bits can simply go into
meson-gxl.dtsi without extra #includes needed in S905X and S905D. While
it's slightly more work to split once again, I think it would be cleaner.

Regards,
Andreas

> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi      | 37 ++++++++++++++++++++
>  arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi  | 43 ++++++++++++++++++++++++
>  arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi |  1 +
>  arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi |  1 +
>  4 files changed, 82 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi
[...]
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
> index 615308e..5a90e30 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
> @@ -42,6 +42,7 @@
>   */
>  
>  #include "meson-gxl.dtsi"
> +#include "meson-gxl-mali.dtsi"
>  
>  / {
>  	compatible = "amlogic,s905d", "amlogic,meson-gxl";
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
> index 08237ee..0f78d83 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
> @@ -42,6 +42,7 @@
>   */
>  
>  #include "meson-gxl.dtsi"
> +#include "meson-gxl-mali.dtsi"
>  
>  / {
>  	compatible = "amlogic,s905x", "amlogic,meson-gxl";

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Neil Armstrong]

Hi Andreas,
On 03/02/2017 01:31 PM, Andreas Färber wrote:
> Hi Neil,
> 
> Am 01.03.2017 um 11:46 schrieb Neil Armstrong:
>> The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs.
> 
> First of all, any reason you're upper-casing Mali in the commit message?
> ARM doesn't.

No reason, only a type, indeed it was lower-casing on the v1.
Will fix in v2.

> 
>>
>> The node is simply added in the meson-gxbb.dtsi file.
> 
> The GXBB part looks fine on a quick look.
> 
>>
>> For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
>> patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
>> dtsi files.
> 
> This part is slightly confusing though.
> 
> What exactly is the GXL vs. GXM difference that this can't be handled by
> overriding node properties compatible/interrupts/clocks? I am missing a
> GXM patch in this series as rationale for doing it this way.
> 
> In particular I am wondering whether the whole GXM-inherits-from-GXL
> concept is flawed and should be adjusted if this leads to secondary
> .dtsi files like this: My proposal would be to instead create a
> meson-gxl-gxm.dtsi, that meson-gxl.dtsi and meson-gxm.dtsi can inherit
> the current common parts from, then the Mali bits can simply go into
> meson-gxl.dtsi without extra #includes needed in S905X and S905D. While
> it's slightly more work to split once again, I think it would be cleaner.

The GXL and GXM differences are very small :
 - They share the same clock tree
 - They share the same pinctrl and even the same pinout (S905D and S912 are pin-to-pin compatible)
 - They share all the peripherals

The only changes are :
 - Enhanced video encoding and decoding support, this will need a family-specific compatible when pushed
 - Slightly differences in the Video Processing Unit, this is why I introduced family-specific compatibles
 - A secondary Cortex-A53 cluster
 - A secondary SCPI cpufreq clock entry
 - A different Mali core, but with the same interrupts (less but they share the same lower interrupts), clocks and memory space

This is why it was decided to have a sub-dtsi, having a secondary dtsi will simply copy 99% of the GXL dtsi,
but surely we could also have an intermediate dtsi but for boards I'm ok with it, but less for a SoC dtsi,
since it could lead to some confusion.

Finally, yes I could have added the mali node to the GXL dtsi, but the midgard Mali dt-bindings are not upstream
and the family is too big and recent enough to consider having stable bindings for now.

Nevertheless, nothing is final, this gxl-mali.dtsi could be merged into the GXL dtsi in the future when we
have proper dt-bindings and a real support of the T820 Mali on the S912.

Kevin, what's your thought about this ?

Neil

> Regards,
> Andreas
> 
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi      | 37 ++++++++++++++++++++
>>  arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi  | 43 ++++++++++++++++++++++++
>>  arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi |  1 +
>>  arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi |  1 +
>>  4 files changed, 82 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi
> [...]
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
>> index 615308e..5a90e30 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
>> @@ -42,6 +42,7 @@
>>   */
>>  
>>  #include "meson-gxl.dtsi"
>> +#include "meson-gxl-mali.dtsi"
>>  
>>  / {
>>  	compatible = "amlogic,s905d", "amlogic,meson-gxl";
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
>> index 08237ee..0f78d83 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi
>> @@ -42,6 +42,7 @@
>>   */
>>  
>>  #include "meson-gxl.dtsi"
>> +#include "meson-gxl-mali.dtsi"
>>  
>>  / {
>>  	compatible = "amlogic,s905x", "amlogic,meson-gxl";
> 

Re: [PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Andreas Färber]

Hi,

Am 02.03.2017 um 13:47 schrieb Neil Armstrong:
> On 03/02/2017 01:31 PM, Andreas Färber wrote:
>> Am 01.03.2017 um 11:46 schrieb Neil Armstrong:
>>> For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
>>> patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
>>> dtsi files.
>>
>> This part is slightly confusing though.
>>
>> What exactly is the GXL vs. GXM difference that this can't be handled by
>> overriding node properties compatible/interrupts/clocks? I am missing a
>> GXM patch in this series as rationale for doing it this way.
>>
>> In particular I am wondering whether the whole GXM-inherits-from-GXL
>> concept is flawed and should be adjusted if this leads to secondary
>> .dtsi files like this: My proposal would be to instead create a
>> meson-gxl-gxm.dtsi, that meson-gxl.dtsi and meson-gxm.dtsi can inherit
>> the current common parts from, then the Mali bits can simply go into
>> meson-gxl.dtsi without extra #includes needed in S905X and S905D. While
>> it's slightly more work to split once again, I think it would be cleaner.
> 
> The GXL and GXM differences are very small :
>  - They share the same clock tree
>  - They share the same pinctrl and even the same pinout (S905D and S912 are pin-to-pin compatible)
>  - They share all the peripherals
> 
> The only changes are :
>  - Enhanced video encoding and decoding support, this will need a family-specific compatible when pushed
>  - Slightly differences in the Video Processing Unit, this is why I introduced family-specific compatibles
>  - A secondary Cortex-A53 cluster
>  - A secondary SCPI cpufreq clock entry
>  - A different Mali core, but with the same interrupts (less but they share the same lower interrupts), clocks and memory space
> 
> This is why it was decided to have a sub-dtsi, having a secondary dtsi will simply copy 99% of the GXL dtsi,
> but surely we could also have an intermediate dtsi but for boards I'm ok with it, but less for a SoC dtsi,
> since it could lead to some confusion.
> 
> Finally, yes I could have added the mali node to the GXL dtsi, but the midgard Mali dt-bindings are not upstream
> and the family is too big and recent enough to consider having stable bindings for now.

OK, my question really was specific to Mali differences. :)

> Nevertheless, nothing is final, this gxl-mali.dtsi could be merged into the GXL dtsi in the future when we
> have proper dt-bindings and a real support of the T820 Mali on the S912.

What about a /delete-node/ &mali; in meson-gxm.dtsi?
That would avoid having any new .dtsi.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Kevin Hilman]

Neil Armstrong <narmstrong@baylibre.com> writes:

> Hi Andreas,
> On 03/02/2017 01:31 PM, Andreas Färber wrote:
>> Hi Neil,
>> 
>> Am 01.03.2017 um 11:46 schrieb Neil Armstrong:
>>> The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs.
>> 
>> First of all, any reason you're upper-casing Mali in the commit message?
>> ARM doesn't.
>
> No reason, only a type, indeed it was lower-casing on the v1.
> Will fix in v2.
>
>> 
>>>
>>> The node is simply added in the meson-gxbb.dtsi file.
>> 
>> The GXBB part looks fine on a quick look.
>> 
>>>
>>> For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
>>> patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
>>> dtsi files.
>> 
>> This part is slightly confusing though.
>> 
>> What exactly is the GXL vs. GXM difference that this can't be handled by
>> overriding node properties compatible/interrupts/clocks? I am missing a
>> GXM patch in this series as rationale for doing it this way.
>> 
>> In particular I am wondering whether the whole GXM-inherits-from-GXL
>> concept is flawed and should be adjusted if this leads to secondary
>> .dtsi files like this: My proposal would be to instead create a
>> meson-gxl-gxm.dtsi, that meson-gxl.dtsi and meson-gxm.dtsi can inherit
>> the current common parts from, then the Mali bits can simply go into
>> meson-gxl.dtsi without extra #includes needed in S905X and S905D. While
>> it's slightly more work to split once again, I think it would be cleaner.
>
> The GXL and GXM differences are very small :
>  - They share the same clock tree
>  - They share the same pinctrl and even the same pinout (S905D and S912 are pin-to-pin compatible)
>  - They share all the peripherals
>
> The only changes are :
>  - Enhanced video encoding and decoding support, this will need a family-specific compatible when pushed
>  - Slightly differences in the Video Processing Unit, this is why I introduced family-specific compatibles
>  - A secondary Cortex-A53 cluster
>  - A secondary SCPI cpufreq clock entry
>  - A different Mali core, but with the same interrupts (less but they share the same lower interrupts), clocks and memory space
>
> This is why it was decided to have a sub-dtsi, having a secondary dtsi will simply copy 99% of the GXL dtsi,
> but surely we could also have an intermediate dtsi but for boards I'm ok with it, but less for a SoC dtsi,
> since it could lead to some confusion.
>
> Finally, yes I could have added the mali node to the GXL dtsi, but the midgard Mali dt-bindings are not upstream
> and the family is too big and recent enough to consider having stable bindings for now.
>
> Nevertheless, nothing is final, this gxl-mali.dtsi could be merged into the GXL dtsi in the future when we
> have proper dt-bindings and a real support of the T820 Mali on the S912.
>
> Kevin, what's your thought about this ?

I don't have a strong preference.  I'm OK with a separate Mali .dtsi due
to the signficant overlap between GXL/GXM in terms of clocks, interrupts
etc.

However, if the plan is to #include this from GXM .dts files, whould a
better name be meson-gx-mali.dtsi?

Kevin

Re: [PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Andreas Färber]

Am 03.03.2017 um 20:29 schrieb Kevin Hilman:
> Neil Armstrong <narmstrong@baylibre.com> writes:
>> On 03/02/2017 01:31 PM, Andreas Färber wrote:
>>> Am 01.03.2017 um 11:46 schrieb Neil Armstrong:
>>>> The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs.
[...]
>>>> The node is simply added in the meson-gxbb.dtsi file.
[...]
>>>> For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
>>>> patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
>>>> dtsi files.
>>>
>>> This part is slightly confusing though.
>>>
>>> What exactly is the GXL vs. GXM difference that this can't be handled by
>>> overriding node properties compatible/interrupts/clocks? I am missing a
>>> GXM patch in this series as rationale for doing it this way.
>>>
>>> In particular I am wondering whether the whole GXM-inherits-from-GXL
>>> concept is flawed and should be adjusted if this leads to secondary
>>> .dtsi files like this: My proposal would be to instead create a
>>> meson-gxl-gxm.dtsi, that meson-gxl.dtsi and meson-gxm.dtsi can inherit
>>> the current common parts from, then the Mali bits can simply go into
>>> meson-gxl.dtsi without extra #includes needed in S905X and S905D. While
>>> it's slightly more work to split once again, I think it would be cleaner.
[...]
>> The only changes are :
[...]
>>  - A different Mali core, but with the same interrupts (less but they share the same lower interrupts), clocks and memory space
>>
>> This is why it was decided to have a sub-dtsi, having a secondary dtsi will simply copy 99% of the GXL dtsi,
>> but surely we could also have an intermediate dtsi but for boards I'm ok with it, but less for a SoC dtsi,
>> since it could lead to some confusion.
>>
>> Finally, yes I could have added the mali node to the GXL dtsi, but the midgard Mali dt-bindings are not upstream
>> and the family is too big and recent enough to consider having stable bindings for now.
>>
>> Nevertheless, nothing is final, this gxl-mali.dtsi could be merged into the GXL dtsi in the future when we
>> have proper dt-bindings and a real support of the T820 Mali on the S912.
>>
>> Kevin, what's your thought about this ?
> 
> I don't have a strong preference.  I'm OK with a separate Mali .dtsi due
> to the signficant overlap between GXL/GXM in terms of clocks, interrupts
> etc.
> 
> However, if the plan is to #include this from GXM .dts files, whould a
> better name be meson-gx-mali.dtsi?

I thought the purpose was specifically to not have GXM include it
because it uses a Midgard IP.

If you want to share the fragment with GXBB too (gx), we should rather
use meson-gx-mali-utgard.dtsi, which would differentiate from GXM's
Midgard while still allowing for variation on the 4xx side (e.g., 470).

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Neil Armstrong]

On 03/04/2017 01:38 PM, Andreas Färber wrote:
> Am 03.03.2017 um 20:29 schrieb Kevin Hilman:
>> Neil Armstrong <narmstrong@baylibre.com> writes:
>>> On 03/02/2017 01:31 PM, Andreas Färber wrote:
>>>> Am 01.03.2017 um 11:46 schrieb Neil Armstrong:
>>>>> The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs.
> [...]
>>>>> The node is simply added in the meson-gxbb.dtsi file.
> [...]
>>>>> For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
>>>>> patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
>>>>> dtsi files.
>>>>
>>>> This part is slightly confusing though.
>>>>
>>>> What exactly is the GXL vs. GXM difference that this can't be handled by
>>>> overriding node properties compatible/interrupts/clocks? I am missing a
>>>> GXM patch in this series as rationale for doing it this way.
>>>>
>>>> In particular I am wondering whether the whole GXM-inherits-from-GXL
>>>> concept is flawed and should be adjusted if this leads to secondary
>>>> .dtsi files like this: My proposal would be to instead create a
>>>> meson-gxl-gxm.dtsi, that meson-gxl.dtsi and meson-gxm.dtsi can inherit
>>>> the current common parts from, then the Mali bits can simply go into
>>>> meson-gxl.dtsi without extra #includes needed in S905X and S905D. While
>>>> it's slightly more work to split once again, I think it would be cleaner.
> [...]
>>> The only changes are :
> [...]
>>>  - A different Mali core, but with the same interrupts (less but they share the same lower interrupts), clocks and memory space
>>>
>>> This is why it was decided to have a sub-dtsi, having a secondary dtsi will simply copy 99% of the GXL dtsi,
>>> but surely we could also have an intermediate dtsi but for boards I'm ok with it, but less for a SoC dtsi,
>>> since it could lead to some confusion.
>>>
>>> Finally, yes I could have added the mali node to the GXL dtsi, but the midgard Mali dt-bindings are not upstream
>>> and the family is too big and recent enough to consider having stable bindings for now.
>>>
>>> Nevertheless, nothing is final, this gxl-mali.dtsi could be merged into the GXL dtsi in the future when we
>>> have proper dt-bindings and a real support of the T820 Mali on the S912.
>>>
>>> Kevin, what's your thought about this ?
>>
>> I don't have a strong preference.  I'm OK with a separate Mali .dtsi due
>> to the signficant overlap between GXL/GXM in terms of clocks, interrupts
>> etc.
>>
>> However, if the plan is to #include this from GXM .dts files, whould a
>> better name be meson-gx-mali.dtsi?
> 
> I thought the purpose was specifically to not have GXM include it
> because it uses a Midgard IP.
> 
> If you want to share the fragment with GXBB too (gx), we should rather
> use meson-gx-mali-utgard.dtsi, which would differentiate from GXM's
> Midgard while still allowing for variation on the 4xx side (e.g., 470).
> 
> Regards,
> Andreas
> 

Exact, there is no plan to include it from GXM.

I'm not fan of having meson-gx-mali-utgard.dtsi, we should still need some attributes additions for
the clocks to the mali node in the gxbb dtsi and each s905x and s905d dtsi files.
I'm not sure this is even cleaner...

Neil

Re: [PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Kevin Hilman]

Neil Armstrong <narmstrong@baylibre.com> writes:

> On 03/04/2017 01:38 PM, Andreas Färber wrote:
>> Am 03.03.2017 um 20:29 schrieb Kevin Hilman:
>>> Neil Armstrong <narmstrong@baylibre.com> writes:
>>>> On 03/02/2017 01:31 PM, Andreas Färber wrote:
>>>>> Am 01.03.2017 um 11:46 schrieb Neil Armstrong:
>>>>>> The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs.
>> [...]
>>>>>> The node is simply added in the meson-gxbb.dtsi file.
>> [...]
>>>>>> For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
>>>>>> patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
>>>>>> dtsi files.
>>>>>
>>>>> This part is slightly confusing though.
>>>>>
>>>>> What exactly is the GXL vs. GXM difference that this can't be handled by
>>>>> overriding node properties compatible/interrupts/clocks? I am missing a
>>>>> GXM patch in this series as rationale for doing it this way.
>>>>>
>>>>> In particular I am wondering whether the whole GXM-inherits-from-GXL
>>>>> concept is flawed and should be adjusted if this leads to secondary
>>>>> .dtsi files like this: My proposal would be to instead create a
>>>>> meson-gxl-gxm.dtsi, that meson-gxl.dtsi and meson-gxm.dtsi can inherit
>>>>> the current common parts from, then the Mali bits can simply go into
>>>>> meson-gxl.dtsi without extra #includes needed in S905X and S905D. While
>>>>> it's slightly more work to split once again, I think it would be cleaner.
>> [...]
>>>> The only changes are :
>> [...]
>>>>  - A different Mali core, but with the same interrupts (less but they share the same lower interrupts), clocks and memory space
>>>>
>>>> This is why it was decided to have a sub-dtsi, having a secondary dtsi will simply copy 99% of the GXL dtsi,
>>>> but surely we could also have an intermediate dtsi but for boards I'm ok with it, but less for a SoC dtsi,
>>>> since it could lead to some confusion.
>>>>
>>>> Finally, yes I could have added the mali node to the GXL dtsi, but the midgard Mali dt-bindings are not upstream
>>>> and the family is too big and recent enough to consider having stable bindings for now.
>>>>
>>>> Nevertheless, nothing is final, this gxl-mali.dtsi could be merged into the GXL dtsi in the future when we
>>>> have proper dt-bindings and a real support of the T820 Mali on the S912.
>>>>
>>>> Kevin, what's your thought about this ?
>>>
>>> I don't have a strong preference.  I'm OK with a separate Mali .dtsi due
>>> to the signficant overlap between GXL/GXM in terms of clocks, interrupts
>>> etc.
>>>
>>> However, if the plan is to #include this from GXM .dts files, whould a
>>> better name be meson-gx-mali.dtsi?
>> 
>> I thought the purpose was specifically to not have GXM include it
>> because it uses a Midgard IP.
>> 
>> If you want to share the fragment with GXBB too (gx), we should rather
>> use meson-gx-mali-utgard.dtsi, which would differentiate from GXM's
>> Midgard while still allowing for variation on the 4xx side (e.g., 470).
>> 
>> Regards,
>> Andreas
>> 
>
> Exact, there is no plan to include it from GXM.
>
> I'm not fan of having meson-gx-mali-utgard.dtsi, we should still need some attributes additions for
> the clocks to the mali node in the gxbb dtsi and each s905x and s905d dtsi files.
> I'm not sure this is even cleaner...

OK, I misunderstood the intent of having it separated from out from the
GXL .dsti then.  Could you please clarify?

Kevin

Re: [PATCH v2 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Neil Armstrong]

On 03/06/2017 06:27 PM, Kevin Hilman wrote:
> Neil Armstrong <narmstrong@baylibre.com> writes:
> 
>> On 03/04/2017 01:38 PM, Andreas Färber wrote:
>>> Am 03.03.2017 um 20:29 schrieb Kevin Hilman:
>>>> Neil Armstrong <narmstrong@baylibre.com> writes:
>>>>> On 03/02/2017 01:31 PM, Andreas Färber wrote:
>>>>>> Am 01.03.2017 um 11:46 schrieb Neil Armstrong:
>>>>>>> The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs.
>>> [...]
>>>>>>> The node is simply added in the meson-gxbb.dtsi file.
>>> [...]
>>>>>>> For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
>>>>>>> patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
>>>>>>> dtsi files.
>>>>>>
>>>>>> This part is slightly confusing though.
>>>>>>
>>>>>> What exactly is the GXL vs. GXM difference that this can't be handled by
>>>>>> overriding node properties compatible/interrupts/clocks? I am missing a
>>>>>> GXM patch in this series as rationale for doing it this way.
>>>>>>
>>>>>> In particular I am wondering whether the whole GXM-inherits-from-GXL
>>>>>> concept is flawed and should be adjusted if this leads to secondary
>>>>>> .dtsi files like this: My proposal would be to instead create a
>>>>>> meson-gxl-gxm.dtsi, that meson-gxl.dtsi and meson-gxm.dtsi can inherit
>>>>>> the current common parts from, then the Mali bits can simply go into
>>>>>> meson-gxl.dtsi without extra #includes needed in S905X and S905D. While
>>>>>> it's slightly more work to split once again, I think it would be cleaner.
>>> [...]
>>>>> The only changes are :
>>> [...]
>>>>>  - A different Mali core, but with the same interrupts (less but they share the same lower interrupts), clocks and memory space
>>>>>
>>>>> This is why it was decided to have a sub-dtsi, having a secondary dtsi will simply copy 99% of the GXL dtsi,
>>>>> but surely we could also have an intermediate dtsi but for boards I'm ok with it, but less for a SoC dtsi,
>>>>> since it could lead to some confusion.
>>>>>
>>>>> Finally, yes I could have added the mali node to the GXL dtsi, but the midgard Mali dt-bindings are not upstream
>>>>> and the family is too big and recent enough to consider having stable bindings for now.
>>>>>
>>>>> Nevertheless, nothing is final, this gxl-mali.dtsi could be merged into the GXL dtsi in the future when we
>>>>> have proper dt-bindings and a real support of the T820 Mali on the S912.
>>>>>
>>>>> Kevin, what's your thought about this ?
>>>>
>>>> I don't have a strong preference.  I'm OK with a separate Mali .dtsi due
>>>> to the signficant overlap between GXL/GXM in terms of clocks, interrupts
>>>> etc.
>>>>
>>>> However, if the plan is to #include this from GXM .dts files, whould a
>>>> better name be meson-gx-mali.dtsi?
>>>
>>> I thought the purpose was specifically to not have GXM include it
>>> because it uses a Midgard IP.
>>>
>>> If you want to share the fragment with GXBB too (gx), we should rather
>>> use meson-gx-mali-utgard.dtsi, which would differentiate from GXM's
>>> Midgard while still allowing for variation on the 4xx side (e.g., 470).
>>>
>>> Regards,
>>> Andreas
>>>
>>
>> Exact, there is no plan to include it from GXM.
>>
>> I'm not fan of having meson-gx-mali-utgard.dtsi, we should still need some attributes additions for
>> the clocks to the mali node in the gxbb dtsi and each s905x and s905d dtsi files.
>> I'm not sure this is even cleaner...
> 
> OK, I misunderstood the intent of having it separated from out from the
> GXL .dsti then.  Could you please clarify?
> 
> Kevin

Hi,

Indeed, my changelog was not really clear, here is the reworded one that will land in v2 :
"
This patch add nodes for the Mali-450 MP3 GPU present in the GXBB and GXL SoCs.

For GXBB, the node is simply added in the meson-gxbb.dtsi file.

Fox GXL, since the GXM dtsi is a superset of the GXL dtsi, the Mali node is added to
the GXL SoC specific dtsi files via an intermediate "meson-gxl-mali.dtsi" file to avoid
having an invalid Mali-450 node in the GXM device tree.
"

Thanks,
Neil