[PATCH v5 0/7] Support for the NPU in Vim3

[PATCH v5 0/7] Support for the NPU in Vim3

[Tomeu Vizoso]

Hi,

This series adds support for the Verisilicon VIPNano-QI NPU in the A311D
as in the VIM3 board.

The IP is very closely based on previous Vivante GPUs, so the etnaviv
kernel driver works basically unchanged.

The userspace part of the driver is being reviewed at:

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18986

v2: Move reference to RESET_NNA to npu node (Neil)
v3: Fix indentation mistake (Neil)
v4: Add warning when etnaviv probes on a NPU (Lucas)
v5: Reorder HWDB commit to be the last (Lucas)

Regards,

Tomeu

Tomeu Vizoso (7):
  dt-bindings: reset: meson-g12a: Add missing NNA reset
  dt-bindings: power: Add G12A NNA power domain
  soc: amlogic: meson-pwrc: Add NNA power domain for A311D
  arm64: dts: Add DT node for the VIPNano-QI on the A311D
  drm/etnaviv: Add nn_core_count to chip feature struct
  drm/etnaviv: Warn when probing on NPUs
  drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055

 .../boot/dts/amlogic/meson-g12-common.dtsi    | 11 ++++++
 .../amlogic/meson-g12b-a311d-khadas-vim3.dts  |  4 +++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c         |  4 +++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h         |  3 ++
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c        | 35 +++++++++++++++++++
 drivers/soc/amlogic/meson-ee-pwrc.c           | 17 +++++++++
 include/dt-bindings/power/meson-g12a-power.h  |  1 +
 .../reset/amlogic,meson-g12a-reset.h          |  4 ++-
 8 files changed, 78 insertions(+), 1 deletion(-)

-- 
2.38.1

[PATCH v5 1/7] dt-bindings: reset: meson-g12a: Add missing NNA reset

[Tomeu Vizoso]

Doesn't appear in the TRM I have, but it is used by the downstream
galcore driver.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Acked-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 include/dt-bindings/reset/amlogic,meson-g12a-reset.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/dt-bindings/reset/amlogic,meson-g12a-reset.h b/include/dt-bindings/reset/amlogic,meson-g12a-reset.h
index 6d487c5eba2c..45f6b8a951d0 100644
--- a/include/dt-bindings/reset/amlogic,meson-g12a-reset.h
+++ b/include/dt-bindings/reset/amlogic,meson-g12a-reset.h
@@ -69,7 +69,9 @@
 #define RESET_PARSER_FETCH		72
 #define RESET_CTL			73
 #define RESET_PARSER_TOP		74
-/*					75-77	*/
+/*					75	*/
+#define RESET_NNA			76
+/*					77	*/
 #define RESET_DVALIN			78
 #define RESET_HDMITX			79
 /*					80-95	*/
-- 
2.38.1

[PATCH v5 2/7] dt-bindings: power: Add G12A NNA power domain

[Tomeu Vizoso]

Add define for the NNA power domain for the NPU in the G12A.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Acked-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 include/dt-bindings/power/meson-g12a-power.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/power/meson-g12a-power.h b/include/dt-bindings/power/meson-g12a-power.h
index bb5e67a842de..93b03bdd60b7 100644
--- a/include/dt-bindings/power/meson-g12a-power.h
+++ b/include/dt-bindings/power/meson-g12a-power.h
@@ -9,5 +9,6 @@
 
 #define PWRC_G12A_VPU_ID		0
 #define PWRC_G12A_ETH_ID		1
+#define PWRC_G12A_NNA_ID		2
 
 #endif
-- 
2.38.1

[PATCH v5 3/7] soc: amlogic: meson-pwrc: Add NNA power domain for A311D

[Tomeu Vizoso]

Based on power initialization sequence in downstream driver.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/soc/amlogic/meson-ee-pwrc.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/soc/amlogic/meson-ee-pwrc.c b/drivers/soc/amlogic/meson-ee-pwrc.c
index dd5f2a13ceb5..dfbf0b1c7d29 100644
--- a/drivers/soc/amlogic/meson-ee-pwrc.c
+++ b/drivers/soc/amlogic/meson-ee-pwrc.c
@@ -46,6 +46,9 @@
 #define HHI_NANOQ_MEM_PD_REG1		(0x47 << 2)
 #define HHI_VPU_MEM_PD_REG2		(0x4d << 2)
 
+#define G12A_HHI_NANOQ_MEM_PD_REG0	(0x43 << 2)
+#define G12A_HHI_NANOQ_MEM_PD_REG1	(0x44 << 2)
+
 struct meson_ee_pwrc;
 struct meson_ee_pwrc_domain;
 
@@ -106,6 +109,13 @@ static struct meson_ee_pwrc_top_domain sm1_pwrc_usb = SM1_EE_PD(17);
 static struct meson_ee_pwrc_top_domain sm1_pwrc_pci = SM1_EE_PD(18);
 static struct meson_ee_pwrc_top_domain sm1_pwrc_ge2d = SM1_EE_PD(19);
 
+static struct meson_ee_pwrc_top_domain g12a_pwrc_nna = {	\
+		.sleep_reg = GX_AO_RTI_GEN_PWR_SLEEP0, 		\
+		.sleep_mask = BIT(16) | BIT(17),		\
+		.iso_reg = GX_AO_RTI_GEN_PWR_ISO0, 		\
+		.iso_mask = BIT(16) | BIT(17), 			\
+	};
+
 /* Memory PD Domains */
 
 #define VPU_MEMPD(__reg)					\
@@ -217,6 +227,11 @@ static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_audio[] = {
 	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(27, 26) },
 };
 
+static struct meson_ee_pwrc_mem_domain g12a_pwrc_mem_nna[] = {
+	{ G12A_HHI_NANOQ_MEM_PD_REG0, GENMASK(31, 0) },
+	{ G12A_HHI_NANOQ_MEM_PD_REG1, GENMASK(23, 0) },
+};
+
 #define VPU_PD(__name, __top_pd, __mem, __is_pwr_off, __resets, __clks)	\
 	{								\
 		.name = __name,						\
@@ -253,6 +268,8 @@ static struct meson_ee_pwrc_domain_desc g12a_pwrc_domains[] = {
 	[PWRC_G12A_VPU_ID]  = VPU_PD("VPU", &gx_pwrc_vpu, g12a_pwrc_mem_vpu,
 				     pwrc_ee_is_powered_off, 11, 2),
 	[PWRC_G12A_ETH_ID] = MEM_PD("ETH", meson_pwrc_mem_eth),
+	[PWRC_G12A_NNA_ID] = TOP_PD("NNA", &g12a_pwrc_nna, g12a_pwrc_mem_nna,
+				    pwrc_ee_is_powered_off),
 };
 
 static struct meson_ee_pwrc_domain_desc gxbb_pwrc_domains[] = {
-- 
2.38.1

[PATCH v5 4/7] arm64: dts: Add DT node for the VIPNano-QI on the A311D

[Tomeu Vizoso]

This "NPU" is very similar to the Vivante GPUs and Etnaviv works well
with it with just a few small changes.

v2: Add reference to RESET_NNA (Neil)
v3: Fix indentation (Neil)

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi     | 11 +++++++++++
 .../boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts |  4 ++++
 2 files changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 45947c1031c4..61c8461df614 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -11,6 +11,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/reset/amlogic,meson-g12a-reset.h>
 #include <dt-bindings/thermal/thermal.h>
+#include <dt-bindings/power/meson-g12a-power.h>
 
 / {
 	interrupt-parent = <&gic>;
@@ -2484,4 +2485,14 @@ xtal: xtal-clk {
 		#clock-cells = <0>;
 	};
 
+	npu: npu@ff100000 {
+		compatible = "vivante,gc";
+		reg = <0x0 0xff100000 0x0 0x20000>;
+		interrupts = <0 147 4>;
+		clocks = <&clkc CLKID_NNA_CORE_CLK>,
+			 <&clkc CLKID_NNA_AXI_CLK>;
+		clock-names = "core", "bus";
+		resets = <&reset RESET_NNA>;
+		power-domains = <&pwrc PWRC_G12A_NNA_ID>;
+	};
 };
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
index 124a80901084..73f3d87dcefd 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-khadas-vim3.dts
@@ -15,6 +15,10 @@ / {
 	compatible = "khadas,vim3", "amlogic,a311d", "amlogic,g12b";
 };
 
+&npu {
+	status = "okay";
+};
+
 /*
  * The VIM3 on-board  MCU can mux the PCIe/USB3.0 shared differential
  * lines using a FUSB340TMX USB 3.1 SuperSpeed Data Switch between
-- 
2.38.1

[PATCH v5 5/7] drm/etnaviv: Add nn_core_count to chip feature struct

[Tomeu Vizoso]

We will use these for differentiating between GPUs and NPUs, as the
downstream driver does.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h  | 3 +++
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 85eddd492774..c8f3ad2031ce 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -50,6 +50,9 @@ struct etnaviv_chip_identity {
 	/* Number of shader cores. */
 	u32 shader_core_count;
 
+	/* Number of Neural Network cores. */
+	u32 nn_core_count;
+
 	/* Size of the vertex cache. */
 	u32 vertex_cache_size;
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index f2fc645c7956..44df273a5aae 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -16,6 +16,7 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
 		.register_max = 64,
 		.thread_count = 128,
 		.shader_core_count = 1,
+		.nn_core_count = 0,
 		.vertex_cache_size = 8,
 		.vertex_output_buffer_size = 1024,
 		.pixel_pipes = 1,
@@ -47,6 +48,7 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
 		.register_max = 64,
 		.thread_count = 512,
 		.shader_core_count = 2,
+		.nn_core_count = 0,
 		.vertex_cache_size = 16,
 		.vertex_output_buffer_size = 1024,
 		.pixel_pipes = 1,
@@ -78,6 +80,7 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
 		.register_max = 64,
 		.thread_count = 512,
 		.shader_core_count = 2,
+		.nn_core_count = 0,
 		.vertex_cache_size = 16,
 		.vertex_output_buffer_size = 1024,
 		.pixel_pipes = 1,
@@ -109,6 +112,7 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
 		.register_max = 64,
 		.thread_count = 1024,
 		.shader_core_count = 4,
+		.nn_core_count = 0,
 		.vertex_cache_size = 16,
 		.vertex_output_buffer_size = 1024,
 		.pixel_pipes = 2,
-- 
2.38.1

[PATCH v5 6/7] drm/etnaviv: Warn when probing on NPUs

[Tomeu Vizoso]

Userspace is still not making full use of the hardware, so we don't know
yet if changes to the UAPI won't be needed. Warn about it.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..3cbc82bbf8d4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -765,6 +765,10 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
 		goto fail;
 	}
 
+	if (gpu->identity.nn_core_count > 0)
+		dev_warn(gpu->dev, "etnaviv has been instantiated on a NPU, "
+                                   "for which the UAPI is still experimental\n");
+
 	/* Exclude VG cores with FE2.0 */
 	if (gpu->identity.features & chipFeatures_PIPE_VG &&
 	    gpu->identity.features & chipFeatures_FE20) {
-- 
2.38.1

[PATCH v5 7/7] drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055

[Tomeu Vizoso]

This is a compute-only module marketed towards AI and vision
acceleration. This particular version can be found on the Amlogic A311D
SoC.

The feature bits are taken from the Khadas downstream kernel driver
6.4.4.3.310723AAA.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 31 ++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index 44df273a5aae..66b8ad6c7d26 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -134,6 +134,37 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
 		.minor_features10 = 0x90044250,
 		.minor_features11 = 0x00000024,
 	},
+	{
+		.model = 0x8000,
+		.revision = 0x7120,
+		.product_id = 0x45080009,
+		.customer_id = 0x88,
+		.eco_id = 0,
+		.stream_count = 8,
+		.register_max = 64,
+		.thread_count = 256,
+		.shader_core_count = 1,
+		.vertex_cache_size = 16,
+		.vertex_output_buffer_size = 1024,
+		.pixel_pipes = 1,
+		.instruction_count = 512,
+		.num_constants = 320,
+		.buffer_size = 0,
+		.varyings_count = 16,
+		.features = 0xe0287cac,
+		.minor_features0 = 0xc1799eff,
+		.minor_features1 = 0xfefbfadb,
+		.minor_features2 = 0xeb9d6fbf,
+		.minor_features3 = 0xedfffced,
+		.minor_features4 = 0xd30dafc7,
+		.minor_features5 = 0x7b5ac333,
+		.minor_features6 = 0xfc8ee200,
+		.minor_features7 = 0x03fffa6f,
+		.minor_features8 = 0x00fe0ef0,
+		.minor_features9 = 0x0088003c,
+		.minor_features10 = 0x108048c0,
+		.minor_features11 = 0x00000010,
+	},
 };
 
 bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
-- 
2.38.1

Re: [PATCH v5 1/7] dt-bindings: reset: meson-g12a: Add missing NNA reset

[Neil Armstrong]

On 01/12/2022 11:30, Tomeu Vizoso wrote:
> Doesn't appear in the TRM I have, but it is used by the downstream
> galcore driver.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Acked-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>   include/dt-bindings/reset/amlogic,meson-g12a-reset.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/dt-bindings/reset/amlogic,meson-g12a-reset.h b/include/dt-bindings/reset/amlogic,meson-g12a-reset.h
> index 6d487c5eba2c..45f6b8a951d0 100644
> --- a/include/dt-bindings/reset/amlogic,meson-g12a-reset.h
> +++ b/include/dt-bindings/reset/amlogic,meson-g12a-reset.h
> @@ -69,7 +69,9 @@
>   #define RESET_PARSER_FETCH		72
>   #define RESET_CTL			73
>   #define RESET_PARSER_TOP		74
> -/*					75-77	*/
> +/*					75	*/
> +#define RESET_NNA			76
> +/*					77	*/
>   #define RESET_DVALIN			78
>   #define RESET_HDMITX			79
>   /*					80-95	*/

Missing from https://lore.kernel.org/all/98517f65e600cddf585e53d1ca98cbb7b0b43aee.camel@pengutronix.de/ :
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

No need to send to send a v6 for that, I'll add it while applying after next rc1.

Neil

Re: [PATCH v5 5/7] drm/etnaviv: Add nn_core_count to chip feature struct

[Lucas Stach]

Am Donnerstag, dem 01.12.2022 um 11:30 +0100 schrieb Tomeu Vizoso:
> We will use these for differentiating between GPUs and NPUs, as the
> downstream driver does.
> 
Thanks, patches 5-7 applied to my etnaviv/next branch.

Regards,
Lucas

> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h  | 3 +++
>  drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 4 ++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 85eddd492774..c8f3ad2031ce 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -50,6 +50,9 @@ struct etnaviv_chip_identity {
>  	/* Number of shader cores. */
>  	u32 shader_core_count;
>  
> +	/* Number of Neural Network cores. */
> +	u32 nn_core_count;
> +
>  	/* Size of the vertex cache. */
>  	u32 vertex_cache_size;
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> index f2fc645c7956..44df273a5aae 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> @@ -16,6 +16,7 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
>  		.register_max = 64,
>  		.thread_count = 128,
>  		.shader_core_count = 1,
> +		.nn_core_count = 0,
>  		.vertex_cache_size = 8,
>  		.vertex_output_buffer_size = 1024,
>  		.pixel_pipes = 1,
> @@ -47,6 +48,7 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
>  		.register_max = 64,
>  		.thread_count = 512,
>  		.shader_core_count = 2,
> +		.nn_core_count = 0,
>  		.vertex_cache_size = 16,
>  		.vertex_output_buffer_size = 1024,
>  		.pixel_pipes = 1,
> @@ -78,6 +80,7 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
>  		.register_max = 64,
>  		.thread_count = 512,
>  		.shader_core_count = 2,
> +		.nn_core_count = 0,
>  		.vertex_cache_size = 16,
>  		.vertex_output_buffer_size = 1024,
>  		.pixel_pipes = 1,
> @@ -109,6 +112,7 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
>  		.register_max = 64,
>  		.thread_count = 1024,
>  		.shader_core_count = 4,
> +		.nn_core_count = 0,
>  		.vertex_cache_size = 16,
>  		.vertex_output_buffer_size = 1024,
>  		.pixel_pipes = 2,

Re: [PATCH v5 1/7] dt-bindings: reset: meson-g12a: Add missing NNA reset

[Martin Blumenstingl]

On Thu, Dec 1, 2022 at 11:30 AM Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>
> Doesn't appear in the TRM I have, but it is used by the downstream
> galcore driver.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Acked-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Re: [PATCH v5 2/7] dt-bindings: power: Add G12A NNA power domain

[Martin Blumenstingl]

On Thu, Dec 1, 2022 at 11:30 AM Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>
> Add define for the NNA power domain for the NPU in the G12A.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Acked-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Re: [PATCH v5 4/7] arm64: dts: Add DT node for the VIPNano-QI on the A311D

[Martin Blumenstingl]

On Thu, Dec 1, 2022 at 11:31 AM Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
[...]
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
> index 45947c1031c4..61c8461df614 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
> @@ -11,6 +11,7 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/reset/amlogic,meson-g12a-reset.h>
>  #include <dt-bindings/thermal/thermal.h>
> +#include <dt-bindings/power/meson-g12a-power.h>
We could now remove the meson-g12a-power include from
arch/arm64/boot/dts/amlogic/meson-g12.dtsi
I'm not sure if we have any "rule" about this though, so I'm curious
to hear what others think.


Best regards,
Martin

Re: [PATCH v5 3/7] soc: amlogic: meson-pwrc: Add NNA power domain for A311D

[Martin Blumenstingl]

On Thu, Dec 1, 2022 at 11:30 AM Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>
> Based on power initialization sequence in downstream driver.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

[...]
> +static struct meson_ee_pwrc_mem_domain g12a_pwrc_mem_nna[] = {
> +       { G12A_HHI_NANOQ_MEM_PD_REG0, GENMASK(31, 0) },
> +       { G12A_HHI_NANOQ_MEM_PD_REG1, GENMASK(23, 0) },
I noticed the discussion in v1 of this series where Neil noted that
you should change GENMASK(31, 0) to GENMASK(23, 0) (for
G12A_HHI_NANOQ_MEM_PD_REG1).
This is all a bit confusing because the S905D3 datasheet mentions that
the HHI_NANOQ_MEM_PD_REG1 register uses the full 32 bits.
I'm still fine with the way it is right now because the datasheets are
not always perfect.


Best regards,
Martin

Re: [PATCH v5 3/7] soc: amlogic: meson-pwrc: Add NNA power domain for A311D

[Neil Armstrong]

On 01/12/2022 23:43, Martin Blumenstingl wrote:
> On Thu, Dec 1, 2022 at 11:30 AM Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>
>> Based on power initialization sequence in downstream driver.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
> [...]
>> +static struct meson_ee_pwrc_mem_domain g12a_pwrc_mem_nna[] = {
>> +       { G12A_HHI_NANOQ_MEM_PD_REG0, GENMASK(31, 0) },
>> +       { G12A_HHI_NANOQ_MEM_PD_REG1, GENMASK(23, 0) },
> I noticed the discussion in v1 of this series where Neil noted that
> you should change GENMASK(31, 0) to GENMASK(23, 0) (for
> G12A_HHI_NANOQ_MEM_PD_REG1).
> This is all a bit confusing because the S905D3 datasheet mentions that
> the HHI_NANOQ_MEM_PD_REG1 register uses the full 32 bits.
> I'm still fine with the way it is right now because the datasheets are
> not always perfect.

Yes they're different in G12B & SM1
Neil

> 
> 
> Best regards,
> Martin

Re: [PATCH v5 4/7] arm64: dts: Add DT node for the VIPNano-QI on the A311D

[Neil Armstrong]

On 01/12/2022 23:33, Martin Blumenstingl wrote:
> On Thu, Dec 1, 2022 at 11:31 AM Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> [...]
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
>> index 45947c1031c4..61c8461df614 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
>> @@ -11,6 +11,7 @@
>>   #include <dt-bindings/interrupt-controller/arm-gic.h>
>>   #include <dt-bindings/reset/amlogic,meson-g12a-reset.h>
>>   #include <dt-bindings/thermal/thermal.h>
>> +#include <dt-bindings/power/meson-g12a-power.h>
> We could now remove the meson-g12a-power include from
> arch/arm64/boot/dts/amlogic/meson-g12.dtsi
> I'm not sure if we have any "rule" about this though, so I'm curious
> to hear what others think.

Actually you're right, Tomeu please remove this include
and remove the:
	power-domains = <&pwrc PWRC_G12A_NNA_ID>;

and move it as:

&npu {
	power-domains = <&pwrc PWRC_G12A_NNA_ID>;
};

into arch/arm64/boot/dts/amlogic/meson-g12b.dtsi

and while you're at it, please add:
&npu {
	power-domains = <&pwrc PWRC_SM1_NNA_ID>;
};

into arch/arm64/boot/dts/amlogic/meson-sm1.dtsi

It's ok to do all this in a single patch.

> 
> 
> Best regards,
> Martin

(PS: Don't forget Philipp's Acked-by on patch 1 for next version)

Thanks,
Neil

Re: [PATCH v5 7/7] drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055

[Lucas Stach]

Hi Tomeu,

Am Donnerstag, dem 01.12.2022 um 11:30 +0100 schrieb Tomeu Vizoso:
> This is a compute-only module marketed towards AI and vision
> acceleration. This particular version can be found on the Amlogic A311D
> SoC.
> 
> The feature bits are taken from the Khadas downstream kernel driver
> 6.4.4.3.310723AAA.
> 
I just noticed that you seem to have dropped the nn_core_count property
from this HWDB entry when reworking the series according to my
feedback. I assume that you are fine with me fixing this up in the
etnaviv tree by using the hunk from your previous patch "[PATCH v4 6/7]
drm/etnaviv: Add nn_core_count to chip feature struct".

If you see any issue with that, please let me know.

Regards,
Lucas

> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 31 ++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> index 44df273a5aae..66b8ad6c7d26 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> @@ -134,6 +134,37 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
>  		.minor_features10 = 0x90044250,
>  		.minor_features11 = 0x00000024,
>  	},
> +	{
> +		.model = 0x8000,
> +		.revision = 0x7120,
> +		.product_id = 0x45080009,
> +		.customer_id = 0x88,
> +		.eco_id = 0,
> +		.stream_count = 8,
> +		.register_max = 64,
> +		.thread_count = 256,
> +		.shader_core_count = 1,
> +		.vertex_cache_size = 16,
> +		.vertex_output_buffer_size = 1024,
> +		.pixel_pipes = 1,
> +		.instruction_count = 512,
> +		.num_constants = 320,
> +		.buffer_size = 0,
> +		.varyings_count = 16,
> +		.features = 0xe0287cac,
> +		.minor_features0 = 0xc1799eff,
> +		.minor_features1 = 0xfefbfadb,
> +		.minor_features2 = 0xeb9d6fbf,
> +		.minor_features3 = 0xedfffced,
> +		.minor_features4 = 0xd30dafc7,
> +		.minor_features5 = 0x7b5ac333,
> +		.minor_features6 = 0xfc8ee200,
> +		.minor_features7 = 0x03fffa6f,
> +		.minor_features8 = 0x00fe0ef0,
> +		.minor_features9 = 0x0088003c,
> +		.minor_features10 = 0x108048c0,
> +		.minor_features11 = 0x00000010,
> +	},
>  };
>  
>  bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)

Re: [PATCH v5 7/7] drm/etnaviv: add HWDB entry for VIPNano-QI.7120.0055

[Tomeu Vizoso]

On 2/1/23 14:26, Lucas Stach wrote:
> Hi Tomeu,
> 
> Am Donnerstag, dem 01.12.2022 um 11:30 +0100 schrieb Tomeu Vizoso:
>> This is a compute-only module marketed towards AI and vision
>> acceleration. This particular version can be found on the Amlogic A311D
>> SoC.
>>
>> The feature bits are taken from the Khadas downstream kernel driver
>> 6.4.4.3.310723AAA.
>>
> I just noticed that you seem to have dropped the nn_core_count property
> from this HWDB entry when reworking the series according to my
> feedback. I assume that you are fine with me fixing this up in the
> etnaviv tree by using the hunk from your previous patch "[PATCH v4 6/7]
> drm/etnaviv: Add nn_core_count to chip feature struct".
> 
> If you see any issue with that, please let me know.

I have checked and I think you are right.

Thanks!

Tomeu

> Regards,
> Lucas
> 
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 31 ++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
>> index 44df273a5aae..66b8ad6c7d26 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
>> @@ -134,6 +134,37 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
>>   		.minor_features10 = 0x90044250,
>>   		.minor_features11 = 0x00000024,
>>   	},
>> +	{
>> +		.model = 0x8000,
>> +		.revision = 0x7120,
>> +		.product_id = 0x45080009,
>> +		.customer_id = 0x88,
>> +		.eco_id = 0,
>> +		.stream_count = 8,
>> +		.register_max = 64,
>> +		.thread_count = 256,
>> +		.shader_core_count = 1,
>> +		.vertex_cache_size = 16,
>> +		.vertex_output_buffer_size = 1024,
>> +		.pixel_pipes = 1,
>> +		.instruction_count = 512,
>> +		.num_constants = 320,
>> +		.buffer_size = 0,
>> +		.varyings_count = 16,
>> +		.features = 0xe0287cac,
>> +		.minor_features0 = 0xc1799eff,
>> +		.minor_features1 = 0xfefbfadb,
>> +		.minor_features2 = 0xeb9d6fbf,
>> +		.minor_features3 = 0xedfffced,
>> +		.minor_features4 = 0xd30dafc7,
>> +		.minor_features5 = 0x7b5ac333,
>> +		.minor_features6 = 0xfc8ee200,
>> +		.minor_features7 = 0x03fffa6f,
>> +		.minor_features8 = 0x00fe0ef0,
>> +		.minor_features9 = 0x0088003c,
>> +		.minor_features10 = 0x108048c0,
>> +		.minor_features11 = 0x00000010,
>> +	},
>>   };
>>   
>>   bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> 

-- 
Tomeu Vizoso
Consultant Principal Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, United 
Kingdom
Registered in England & Wales, no. 5513718