[PATCH 0/3] DRM/MSM: Support for Adreno 663 GPU

[PATCH 0/3] DRM/MSM: Support for Adreno 663 GPU

[Akhil P Oommen]

This series adds support for Adreno 663 gpu found in SA8775P chipsets.
The closest gpu which is currently supported in drm-msm is A660.
Following are the major differences with that:
	1. gmu/zap firmwares
	2. Recommended to disable Level2 swizzling

Verified kmscube with the below Mesa change [1]. This series is rebased
on top of msm-next.

Patch (1) & (2) for Rob Clark and Patch (3) for Bjorn

[0] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/31211

To: Rob Clark <robdclark@gmail.com>
To: Sean Paul <sean@poorly.run>
To: Konrad Dybcio <konrad.dybcio@linaro.org>
To: Abhinav Kumar <quic_abhinavk@quicinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Marijn Suijten <marijn.suijten@somainline.org>
To: David Airlie <airlied@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
Puranam V G Tejaswi (3):
      drm/msm/a6xx: Add support for A663
      dt-bindings: display/msm/gmu: Add Adreno 663 GMU
      arm64: dts: qcom: sa8775p: Add gpu and gmu nodes

 .../devicetree/bindings/display/msm/gmu.yaml       |  1 +
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi         |  8 +++
 arch/arm64/boot/dts/qcom/sa8775p.dtsi              | 75 ++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/a6xx_catalog.c          | 19 ++++++
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c              |  8 ++-
 drivers/gpu/drm/msm/adreno/a6xx_hfi.c              | 33 ++++++++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.h            |  5 ++
 7 files changed, 148 insertions(+), 1 deletion(-)
---
base-commit: 15302579373ed2c8ada629e9e7bcf9569393a48d
change-id: 20240917-a663-gpu-support-b1475c828606

Best regards,
-- 
Akhil P Oommen <quic_akhilpo@quicinc.com>

[PATCH 1/3] drm/msm/a6xx: Add support for A663

[Akhil P Oommen]

From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>

Add support for Adreno 663 found on sa8775p based platforms.

Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 19 ++++++++++++++++++
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c     |  8 +++++++-
 drivers/gpu/drm/msm/adreno/a6xx_hfi.c     | 33 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.h   |  5 +++++
 4 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
index 0312b6ee0356..8d8d0d7630f0 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
@@ -972,6 +972,25 @@ static const struct adreno_info a6xx_gpus[] = {
 			.prim_fifo_threshold = 0x00300200,
 		},
 		.address_space_size = SZ_16G,
+	}, {
+		.chip_ids = ADRENO_CHIP_IDS(0x06060300),
+		.family = ADRENO_6XX_GEN4,
+		.fw = {
+			[ADRENO_FW_SQE] = "a660_sqe.fw",
+			[ADRENO_FW_GMU] = "a663_gmu.bin",
+		},
+		.gmem = SZ_1M + SZ_512K,
+		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
+		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
+			ADRENO_QUIRK_HAS_HW_APRIV,
+		.init = a6xx_gpu_init,
+		.a6xx = &(const struct a6xx_info) {
+			.hwcg = a690_hwcg,
+			.protect = &a660_protect,
+			.gmu_cgc_mode = 0x00020200,
+			.prim_fifo_threshold = 0x00300200,
+		},
+		.address_space_size = SZ_16G,
 	}, {
 		.chip_ids = ADRENO_CHIP_IDS(0x06030500),
 		.family = ADRENO_6XX_GEN4,
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 06cab2c6fd66..e317780caeae 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -541,6 +541,12 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu)
 		gpu->ubwc_config.macrotile_mode = 1;
 	}
 
+	if (adreno_is_a663(gpu)) {
+		gpu->ubwc_config.highest_bank_bit = 13;
+		gpu->ubwc_config.ubwc_swizzle = 0x4;
+		gpu->ubwc_config.macrotile_mode = 1;
+	}
+
 	if (adreno_is_7c3(gpu)) {
 		gpu->ubwc_config.highest_bank_bit = 14;
 		gpu->ubwc_config.amsbc = 1;
@@ -1062,7 +1068,7 @@ static int hw_init(struct msm_gpu *gpu)
 	if (adreno_is_a690(adreno_gpu))
 		gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x90);
 	/* Set dualQ + disable afull for A660 GPU */
-	else if (adreno_is_a660(adreno_gpu))
+	else if (adreno_is_a660(adreno_gpu) || adreno_is_a663(adreno_gpu))
 		gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x66906);
 	else if (adreno_is_a7xx(adreno_gpu))
 		gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG,
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
index cdb3f6e74d3e..f1196d66055c 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
@@ -478,6 +478,37 @@ static void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
 	msg->cnoc_cmds_data[1][0] =  0x60000001;
 }
 
+static void a663_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
+{
+	/*
+	 * Send a single "off" entry just to get things running
+	 * TODO: bus scaling
+	 */
+	msg->bw_level_num = 1;
+
+	msg->ddr_cmds_num = 3;
+	msg->ddr_wait_bitmask = 0x07;
+
+	msg->ddr_cmds_addrs[0] = 0x50004;
+	msg->ddr_cmds_addrs[1] = 0x50000;
+	msg->ddr_cmds_addrs[2] = 0x500b4;
+
+	msg->ddr_cmds_data[0][0] =  0x40000000;
+	msg->ddr_cmds_data[0][1] =  0x40000000;
+	msg->ddr_cmds_data[0][2] =  0x40000000;
+
+	/*
+	 * These are the CX (CNOC) votes - these are used by the GMU but the
+	 * votes are known and fixed for the target
+	 */
+	msg->cnoc_cmds_num = 1;
+	msg->cnoc_wait_bitmask = 0x01;
+
+	msg->cnoc_cmds_addrs[0] = 0x50058;
+	msg->cnoc_cmds_data[0][0] =  0x40000000;
+	msg->cnoc_cmds_data[1][0] =  0x60000001;
+}
+
 static void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
 {
 	/*
@@ -646,6 +677,8 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu)
 		adreno_7c3_build_bw_table(&msg);
 	else if (adreno_is_a660(adreno_gpu))
 		a660_build_bw_table(&msg);
+	else if (adreno_is_a663(adreno_gpu))
+		a663_build_bw_table(&msg);
 	else if (adreno_is_a690(adreno_gpu))
 		a690_build_bw_table(&msg);
 	else if (adreno_is_a730(adreno_gpu))
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 58d7e7915c57..10f8f25d8826 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -455,6 +455,11 @@ static inline int adreno_is_a680(const struct adreno_gpu *gpu)
 	return adreno_is_revn(gpu, 680);
 }
 
+static inline int adreno_is_a663(const struct adreno_gpu *gpu)
+{
+	return gpu->info->chip_ids[0] == 0x06060300;
+}
+
 static inline int adreno_is_a690(const struct adreno_gpu *gpu)
 {
 	return gpu->info->chip_ids[0] == 0x06090000;

-- 
2.45.2

[PATCH 2/3] dt-bindings: display/msm/gmu: Add Adreno 663 GMU

[Akhil P Oommen]

From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>

Document Adreno 663 GMU in the dt-binding specification.

Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 Documentation/devicetree/bindings/display/msm/gmu.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml b/Documentation/devicetree/bindings/display/msm/gmu.yaml
index b1bd372996d5..ab884e236429 100644
--- a/Documentation/devicetree/bindings/display/msm/gmu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml
@@ -125,6 +125,7 @@ allOf:
             enum:
               - qcom,adreno-gmu-635.0
               - qcom,adreno-gmu-660.1
+              - qcom,adreno-gmu-663.0
     then:
       properties:
         reg:

-- 
2.45.2

[PATCH 3/3] arm64: dts: qcom: sa8775p: Add gpu and gmu nodes

[Akhil P Oommen]

From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>

Add gpu and gmu nodes for sa8775p based platforms.

Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi |  8 ++++
 arch/arm64/boot/dts/qcom/sa8775p.dtsi      | 75 ++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
index 2a6170623ea9..a01e6675c4bb 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
@@ -407,6 +407,14 @@ queue3 {
 	};
 };
 
+&gpu {
+	status = "okay";
+
+	zap-shader {
+		firmware-name = "qcom/sa8775p/a663_zap.mbn";
+	};
+};
+
 &i2c11 {
 	clock-frequency = <400000>;
 	pinctrl-0 = <&qup_i2c11_default>;
diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index 23f1b2e5e624..12c79135a303 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -2824,6 +2824,81 @@ tcsr_mutex: hwlock@1f40000 {
 			#hwlock-cells = <1>;
 		};
 
+		gpu: gpu@3d00000 {
+			compatible = "qcom,adreno-663.0", "qcom,adreno";
+			reg = <0 0x03d00000 0 0x40000>,
+			      <0 0x03d9e000 0 0x1000>,
+			      <0 0x03d61000 0 0x800>;
+			reg-names = "kgsl_3d0_reg_memory",
+				    "cx_mem",
+				    "cx_dbgc";
+			interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
+			iommus = <&adreno_smmu 0 0xc00>,
+				 <&adreno_smmu 1 0xc00>;
+			operating-points-v2 = <&gpu_opp_table>;
+			qcom,gmu = <&gmu>;
+			interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;
+			interconnect-names = "gfx-mem";
+			#cooling-cells = <2>;
+
+			status = "disabled";
+
+			zap-shader {
+				memory-region = <&pil_gpu_mem>;
+			};
+
+			gpu_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-405000000 {
+					opp-hz = /bits/ 64 <405000000>;
+					opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
+					opp-peak-kBps = <8368000>;
+				};
+
+			};
+		};
+
+		gmu: gmu@3d6a000 {
+			compatible = "qcom,adreno-gmu-663.0", "qcom,adreno-gmu";
+			reg = <0 0x03d6a000 0 0x34000>,
+				<0 0x3de0000 0 0x10000>,
+				<0 0x0b290000 0 0x10000>;
+			reg-names = "gmu", "rscc", "gmu_pdc";
+			interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hfi", "gmu";
+			clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
+				 <&gpucc GPU_CC_CXO_CLK>,
+				 <&gcc GCC_DDRSS_GPU_AXI_CLK>,
+				 <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
+				 <&gpucc GPU_CC_AHB_CLK>,
+				 <&gpucc GPU_CC_HUB_CX_INT_CLK>,
+				 <&gpucc GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK>;
+			clock-names = "gmu",
+				      "cxo",
+				      "axi",
+				      "memnoc",
+				      "ahb",
+				      "hub",
+				      "smmu_vote";
+			power-domains = <&gpucc GPU_CC_CX_GDSC>,
+					<&gpucc GPU_CC_GX_GDSC>;
+			power-domain-names = "cx",
+					     "gx";
+			iommus = <&adreno_smmu 5 0xc00>;
+			operating-points-v2 = <&gmu_opp_table>;
+
+			gmu_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-200000000 {
+					opp-hz = /bits/ 64 <200000000>;
+					opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
+				};
+			};
+		};
+
 		gpucc: clock-controller@3d90000 {
 			compatible = "qcom,sa8775p-gpucc";
 			reg = <0x0 0x03d90000 0x0 0xa000>;

-- 
2.45.2

Re: [PATCH 3/3] arm64: dts: qcom: sa8775p: Add gpu and gmu nodes

[Dmitry Baryshkov]

On Wed, Sep 18, 2024 at 02:08:43AM GMT, Akhil P Oommen wrote:
> From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> 
> Add gpu and gmu nodes for sa8775p based platforms.

Which platforms? The commit adds nodes to the SoC and the single RIDE
platform.

> 
> Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi |  8 ++++
>  arch/arm64/boot/dts/qcom/sa8775p.dtsi      | 75 ++++++++++++++++++++++++++++++
>  2 files changed, 83 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> index 2a6170623ea9..a01e6675c4bb 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> @@ -407,6 +407,14 @@ queue3 {
>  	};
>  };
>  
> +&gpu {
> +	status = "okay";
> +
> +	zap-shader {

It's easier to add gpu_zap_shader_link label in the DTSI file and then
reference it instead of using the subnode again.

> +		firmware-name = "qcom/sa8775p/a663_zap.mbn";
> +	};
> +};

Separate patch, please.

> +
>  &i2c11 {
>  	clock-frequency = <400000>;
>  	pinctrl-0 = <&qup_i2c11_default>;
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> index 23f1b2e5e624..12c79135a303 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> @@ -2824,6 +2824,81 @@ tcsr_mutex: hwlock@1f40000 {
>  			#hwlock-cells = <1>;
>  		};
>  
> +		gpu: gpu@3d00000 {
> +			compatible = "qcom,adreno-663.0", "qcom,adreno";
> +			reg = <0 0x03d00000 0 0x40000>,
> +			      <0 0x03d9e000 0 0x1000>,
> +			      <0 0x03d61000 0 0x800>;

I think it's suggested to use 0x0 now

> +			reg-names = "kgsl_3d0_reg_memory",
> +				    "cx_mem",
> +				    "cx_dbgc";
> +			interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> +			iommus = <&adreno_smmu 0 0xc00>,
> +				 <&adreno_smmu 1 0xc00>;
> +			operating-points-v2 = <&gpu_opp_table>;
> +			qcom,gmu = <&gmu>;
> +			interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;

QCOM_ICC_TAG_ALWAYS instead of 0

> +			interconnect-names = "gfx-mem";
> +			#cooling-cells = <2>;

No speed bins?

> +
> +			status = "disabled";
> +
> +			zap-shader {

gpu_zap_shader: zap-shader

> +				memory-region = <&pil_gpu_mem>;
> +			};
> +
> +			gpu_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +
> +				opp-405000000 {

Just a single freq?

> +					opp-hz = /bits/ 64 <405000000>;
> +					opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> +					opp-peak-kBps = <8368000>;
> +				};
> +

Drop the empty line, please.

> +			};
> +		};
> +
> +		gmu: gmu@3d6a000 {
> +			compatible = "qcom,adreno-gmu-663.0", "qcom,adreno-gmu";
> +			reg = <0 0x03d6a000 0 0x34000>,
> +				<0 0x3de0000 0 0x10000>,
> +				<0 0x0b290000 0 0x10000>;

Wrong indentation, please align to the angle bracket.
Also I think it's suggested to use 0x0 now

> +			reg-names = "gmu", "rscc", "gmu_pdc";
> +			interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;

And here

> +			interrupt-names = "hfi", "gmu";
> +			clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
> +				 <&gpucc GPU_CC_CXO_CLK>,
> +				 <&gcc GCC_DDRSS_GPU_AXI_CLK>,
> +				 <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
> +				 <&gpucc GPU_CC_AHB_CLK>,
> +				 <&gpucc GPU_CC_HUB_CX_INT_CLK>,
> +				 <&gpucc GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK>;
> +			clock-names = "gmu",
> +				      "cxo",
> +				      "axi",
> +				      "memnoc",
> +				      "ahb",
> +				      "hub",
> +				      "smmu_vote";
> +			power-domains = <&gpucc GPU_CC_CX_GDSC>,
> +					<&gpucc GPU_CC_GX_GDSC>;
> +			power-domain-names = "cx",
> +					     "gx";
> +			iommus = <&adreno_smmu 5 0xc00>;
> +			operating-points-v2 = <&gmu_opp_table>;
> +
> +			gmu_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +
> +				opp-200000000 {
> +					opp-hz = /bits/ 64 <200000000>;
> +					opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> +				};
> +			};
> +		};
> +
>  		gpucc: clock-controller@3d90000 {
>  			compatible = "qcom,sa8775p-gpucc";
>  			reg = <0x0 0x03d90000 0x0 0xa000>;
> 
> -- 
> 2.45.2
> 

-- 
With best wishes
Dmitry

Re: [PATCH 1/3] drm/msm/a6xx: Add support for A663

[Dmitry Baryshkov]

On Wed, Sep 18, 2024 at 02:08:41AM GMT, Akhil P Oommen wrote:
> From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> 
> Add support for Adreno 663 found on sa8775p based platforms.
> 
> Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 19 ++++++++++++++++++
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c     |  8 +++++++-
>  drivers/gpu/drm/msm/adreno/a6xx_hfi.c     | 33 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h   |  5 +++++
>  4 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> index 0312b6ee0356..8d8d0d7630f0 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> @@ -972,6 +972,25 @@ static const struct adreno_info a6xx_gpus[] = {
>  			.prim_fifo_threshold = 0x00300200,
>  		},
>  		.address_space_size = SZ_16G,
> +	}, {
> +		.chip_ids = ADRENO_CHIP_IDS(0x06060300),
> +		.family = ADRENO_6XX_GEN4,
> +		.fw = {
> +			[ADRENO_FW_SQE] = "a660_sqe.fw",
> +			[ADRENO_FW_GMU] = "a663_gmu.bin",
> +		},
> +		.gmem = SZ_1M + SZ_512K,
> +		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +			ADRENO_QUIRK_HAS_HW_APRIV,
> +		.init = a6xx_gpu_init,
> +		.a6xx = &(const struct a6xx_info) {
> +			.hwcg = a690_hwcg,
> +			.protect = &a660_protect,
> +			.gmu_cgc_mode = 0x00020200,
> +			.prim_fifo_threshold = 0x00300200,
> +		},
> +		.address_space_size = SZ_16G,
>  	}, {
>  		.chip_ids = ADRENO_CHIP_IDS(0x06030500),
>  		.family = ADRENO_6XX_GEN4,
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 06cab2c6fd66..e317780caeae 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -541,6 +541,12 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu)
>  		gpu->ubwc_config.macrotile_mode = 1;
>  	}
>  
> +	if (adreno_is_a663(gpu)) {
> +		gpu->ubwc_config.highest_bank_bit = 13;
> +		gpu->ubwc_config.ubwc_swizzle = 0x4;
> +		gpu->ubwc_config.macrotile_mode = 1;

If this looks like A660 / A690, shouldn't the driver also enable .amsbc,
.rgb565_predicator and .uavflagprd_inv?

> +	}
> +
>  	if (adreno_is_7c3(gpu)) {
>  		gpu->ubwc_config.highest_bank_bit = 14;
>  		gpu->ubwc_config.amsbc = 1;
> @@ -1062,7 +1068,7 @@ static int hw_init(struct msm_gpu *gpu)
>  	if (adreno_is_a690(adreno_gpu))
>  		gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x90);
>  	/* Set dualQ + disable afull for A660 GPU */
> -	else if (adreno_is_a660(adreno_gpu))
> +	else if (adreno_is_a660(adreno_gpu) || adreno_is_a663(adreno_gpu))
>  		gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x66906);
>  	else if (adreno_is_a7xx(adreno_gpu))
>  		gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG,
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> index cdb3f6e74d3e..f1196d66055c 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> @@ -478,6 +478,37 @@ static void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[1][0] =  0x60000001;
>  }
>  
> +static void a663_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +{
> +	/*
> +	 * Send a single "off" entry just to get things running
> +	 * TODO: bus scaling
> +	 */
> +	msg->bw_level_num = 1;
> +
> +	msg->ddr_cmds_num = 3;
> +	msg->ddr_wait_bitmask = 0x07;
> +
> +	msg->ddr_cmds_addrs[0] = 0x50004;
> +	msg->ddr_cmds_addrs[1] = 0x50000;
> +	msg->ddr_cmds_addrs[2] = 0x500b4;
> +
> +	msg->ddr_cmds_data[0][0] =  0x40000000;
> +	msg->ddr_cmds_data[0][1] =  0x40000000;
> +	msg->ddr_cmds_data[0][2] =  0x40000000;
> +
> +	/*
> +	 * These are the CX (CNOC) votes - these are used by the GMU but the
> +	 * votes are known and fixed for the target
> +	 */
> +	msg->cnoc_cmds_num = 1;
> +	msg->cnoc_wait_bitmask = 0x01;
> +
> +	msg->cnoc_cmds_addrs[0] = 0x50058;
> +	msg->cnoc_cmds_data[0][0] =  0x40000000;
> +	msg->cnoc_cmds_data[1][0] =  0x60000001;
> +}
> +
>  static void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	/*
> @@ -646,6 +677,8 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu)
>  		adreno_7c3_build_bw_table(&msg);
>  	else if (adreno_is_a660(adreno_gpu))
>  		a660_build_bw_table(&msg);
> +	else if (adreno_is_a663(adreno_gpu))
> +		a663_build_bw_table(&msg);
>  	else if (adreno_is_a690(adreno_gpu))
>  		a690_build_bw_table(&msg);
>  	else if (adreno_is_a730(adreno_gpu))
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 58d7e7915c57..10f8f25d8826 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -455,6 +455,11 @@ static inline int adreno_is_a680(const struct adreno_gpu *gpu)
>  	return adreno_is_revn(gpu, 680);
>  }
>  
> +static inline int adreno_is_a663(const struct adreno_gpu *gpu)
> +{
> +	return gpu->info->chip_ids[0] == 0x06060300;
> +}
> +
>  static inline int adreno_is_a690(const struct adreno_gpu *gpu)
>  {
>  	return gpu->info->chip_ids[0] == 0x06090000;
> 
> -- 
> 2.45.2
> 

-- 
With best wishes
Dmitry

Re: [PATCH 0/3] DRM/MSM: Support for Adreno 663 GPU

[Dmitry Baryshkov]

On Wed, Sep 18, 2024 at 02:08:40AM GMT, Akhil P Oommen wrote:
> This series adds support for Adreno 663 gpu found in SA8775P chipsets.
> The closest gpu which is currently supported in drm-msm is A660.
> Following are the major differences with that:
> 	1. gmu/zap firmwares
> 	2. Recommended to disable Level2 swizzling
> 
> Verified kmscube with the below Mesa change [1]. This series is rebased
> on top of msm-next.

Is there a chance of you sharing Vulkan CTS results?

> 
> Patch (1) & (2) for Rob Clark and Patch (3) for Bjorn
> 
> [0] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/31211
> 
> To: Rob Clark <robdclark@gmail.com>
> To: Sean Paul <sean@poorly.run>
> To: Konrad Dybcio <konrad.dybcio@linaro.org>
> To: Abhinav Kumar <quic_abhinavk@quicinc.com>
> To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> To: Marijn Suijten <marijn.suijten@somainline.org>
> To: David Airlie <airlied@gmail.com>
> To: Daniel Vetter <daniel@ffwll.ch>
> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> To: Maxime Ripard <mripard@kernel.org>
> To: Thomas Zimmermann <tzimmermann@suse.de>
> To: Rob Herring <robh@kernel.org>
> To: Krzysztof Kozlowski <krzk+dt@kernel.org>
> To: Conor Dooley <conor+dt@kernel.org>
> To: Bjorn Andersson <andersson@kernel.org>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: freedreno@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> 
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
> Puranam V G Tejaswi (3):
>       drm/msm/a6xx: Add support for A663
>       dt-bindings: display/msm/gmu: Add Adreno 663 GMU
>       arm64: dts: qcom: sa8775p: Add gpu and gmu nodes
> 
>  .../devicetree/bindings/display/msm/gmu.yaml       |  1 +
>  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi         |  8 +++
>  arch/arm64/boot/dts/qcom/sa8775p.dtsi              | 75 ++++++++++++++++++++++
>  drivers/gpu/drm/msm/adreno/a6xx_catalog.c          | 19 ++++++
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c              |  8 ++-
>  drivers/gpu/drm/msm/adreno/a6xx_hfi.c              | 33 ++++++++++
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h            |  5 ++
>  7 files changed, 148 insertions(+), 1 deletion(-)
> ---
> base-commit: 15302579373ed2c8ada629e9e7bcf9569393a48d
> change-id: 20240917-a663-gpu-support-b1475c828606
> 
> Best regards,
> -- 
> Akhil P Oommen <quic_akhilpo@quicinc.com>
> 

-- 
With best wishes
Dmitry

Re: [PATCH 2/3] dt-bindings: display/msm/gmu: Add Adreno 663 GMU

[Rob Herring (Arm)]

On Wed, 18 Sep 2024 02:08:42 +0530, Akhil P Oommen wrote:
> From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> 
> Document Adreno 663 GMU in the dt-binding specification.
> 
> Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>  Documentation/devicetree/bindings/display/msm/gmu.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 1/3] drm/msm/a6xx: Add support for A663

[Connor Abbott]

On Tue, Sep 17, 2024 at 9:39 PM Akhil P Oommen <quic_akhilpo@quicinc.com> wrote:
>
> From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
>
> Add support for Adreno 663 found on sa8775p based platforms.
>
> Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 19 ++++++++++++++++++
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c     |  8 +++++++-
>  drivers/gpu/drm/msm/adreno/a6xx_hfi.c     | 33 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h   |  5 +++++
>  4 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> index 0312b6ee0356..8d8d0d7630f0 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> @@ -972,6 +972,25 @@ static const struct adreno_info a6xx_gpus[] = {
>                         .prim_fifo_threshold = 0x00300200,
>                 },
>                 .address_space_size = SZ_16G,
> +       }, {
> +               .chip_ids = ADRENO_CHIP_IDS(0x06060300),
> +               .family = ADRENO_6XX_GEN4,
> +               .fw = {
> +                       [ADRENO_FW_SQE] = "a660_sqe.fw",
> +                       [ADRENO_FW_GMU] = "a663_gmu.bin",
> +               },
> +               .gmem = SZ_1M + SZ_512K,
> +               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> +               .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> +                       ADRENO_QUIRK_HAS_HW_APRIV,
> +               .init = a6xx_gpu_init,
> +               .a6xx = &(const struct a6xx_info) {
> +                       .hwcg = a690_hwcg,
> +                       .protect = &a660_protect,
> +                       .gmu_cgc_mode = 0x00020200,
> +                       .prim_fifo_threshold = 0x00300200,
> +               },
> +               .address_space_size = SZ_16G,
>         }, {
>                 .chip_ids = ADRENO_CHIP_IDS(0x06030500),
>                 .family = ADRENO_6XX_GEN4,
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 06cab2c6fd66..e317780caeae 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -541,6 +541,12 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu)
>                 gpu->ubwc_config.macrotile_mode = 1;
>         }
>
> +       if (adreno_is_a663(gpu)) {
> +               gpu->ubwc_config.highest_bank_bit = 13;
> +               gpu->ubwc_config.ubwc_swizzle = 0x4;

It's already been mentioned in the Mesa MR, but since this is the
first GPU with level2_swizzling_dis set, the relevant vulkan CTS tests
need to be tested with
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26578
rebased on your Mesa enablement patch.

> +               gpu->ubwc_config.macrotile_mode = 1;
> +       }
> +
>         if (adreno_is_7c3(gpu)) {
>                 gpu->ubwc_config.highest_bank_bit = 14;
>                 gpu->ubwc_config.amsbc = 1;
> @@ -1062,7 +1068,7 @@ static int hw_init(struct msm_gpu *gpu)
>         if (adreno_is_a690(adreno_gpu))
>                 gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x90);
>         /* Set dualQ + disable afull for A660 GPU */
> -       else if (adreno_is_a660(adreno_gpu))
> +       else if (adreno_is_a660(adreno_gpu) || adreno_is_a663(adreno_gpu))
>                 gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x66906);
>         else if (adreno_is_a7xx(adreno_gpu))
>                 gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG,
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> index cdb3f6e74d3e..f1196d66055c 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> @@ -478,6 +478,37 @@ static void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>         msg->cnoc_cmds_data[1][0] =  0x60000001;
>  }
>
> +static void a663_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +{
> +       /*
> +        * Send a single "off" entry just to get things running
> +        * TODO: bus scaling
> +        */
> +       msg->bw_level_num = 1;
> +
> +       msg->ddr_cmds_num = 3;
> +       msg->ddr_wait_bitmask = 0x07;
> +
> +       msg->ddr_cmds_addrs[0] = 0x50004;
> +       msg->ddr_cmds_addrs[1] = 0x50000;
> +       msg->ddr_cmds_addrs[2] = 0x500b4;
> +
> +       msg->ddr_cmds_data[0][0] =  0x40000000;
> +       msg->ddr_cmds_data[0][1] =  0x40000000;
> +       msg->ddr_cmds_data[0][2] =  0x40000000;
> +
> +       /*
> +        * These are the CX (CNOC) votes - these are used by the GMU but the
> +        * votes are known and fixed for the target
> +        */
> +       msg->cnoc_cmds_num = 1;
> +       msg->cnoc_wait_bitmask = 0x01;
> +
> +       msg->cnoc_cmds_addrs[0] = 0x50058;
> +       msg->cnoc_cmds_data[0][0] =  0x40000000;
> +       msg->cnoc_cmds_data[1][0] =  0x60000001;
> +}
> +
>  static void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>         /*
> @@ -646,6 +677,8 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu)
>                 adreno_7c3_build_bw_table(&msg);
>         else if (adreno_is_a660(adreno_gpu))
>                 a660_build_bw_table(&msg);
> +       else if (adreno_is_a663(adreno_gpu))
> +               a663_build_bw_table(&msg);
>         else if (adreno_is_a690(adreno_gpu))
>                 a690_build_bw_table(&msg);
>         else if (adreno_is_a730(adreno_gpu))
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 58d7e7915c57..10f8f25d8826 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -455,6 +455,11 @@ static inline int adreno_is_a680(const struct adreno_gpu *gpu)
>         return adreno_is_revn(gpu, 680);
>  }
>
> +static inline int adreno_is_a663(const struct adreno_gpu *gpu)
> +{
> +       return gpu->info->chip_ids[0] == 0x06060300;
> +}
> +
>  static inline int adreno_is_a690(const struct adreno_gpu *gpu)
>  {
>         return gpu->info->chip_ids[0] == 0x06090000;
>
> --
> 2.45.2
>

Re: [PATCH 1/3] drm/msm/a6xx: Add support for A663

[Akhil P Oommen]

On Wed, Sep 18, 2024 at 12:31:55AM +0300, Dmitry Baryshkov wrote:
> On Wed, Sep 18, 2024 at 02:08:41AM GMT, Akhil P Oommen wrote:
> > From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> > 
> > Add support for Adreno 663 found on sa8775p based platforms.
> > 
> > Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> > ---
> >  drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 19 ++++++++++++++++++
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c     |  8 +++++++-
> >  drivers/gpu/drm/msm/adreno/a6xx_hfi.c     | 33 +++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.h   |  5 +++++
> >  4 files changed, 64 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> > index 0312b6ee0356..8d8d0d7630f0 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> > @@ -972,6 +972,25 @@ static const struct adreno_info a6xx_gpus[] = {
> >  			.prim_fifo_threshold = 0x00300200,
> >  		},
> >  		.address_space_size = SZ_16G,
> > +	}, {
> > +		.chip_ids = ADRENO_CHIP_IDS(0x06060300),
> > +		.family = ADRENO_6XX_GEN4,
> > +		.fw = {
> > +			[ADRENO_FW_SQE] = "a660_sqe.fw",
> > +			[ADRENO_FW_GMU] = "a663_gmu.bin",
> > +		},
> > +		.gmem = SZ_1M + SZ_512K,
> > +		.inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > +		.quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > +			ADRENO_QUIRK_HAS_HW_APRIV,
> > +		.init = a6xx_gpu_init,
> > +		.a6xx = &(const struct a6xx_info) {
> > +			.hwcg = a690_hwcg,
> > +			.protect = &a660_protect,
> > +			.gmu_cgc_mode = 0x00020200,
> > +			.prim_fifo_threshold = 0x00300200,
> > +		},
> > +		.address_space_size = SZ_16G,
> >  	}, {
> >  		.chip_ids = ADRENO_CHIP_IDS(0x06030500),
> >  		.family = ADRENO_6XX_GEN4,
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > index 06cab2c6fd66..e317780caeae 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > @@ -541,6 +541,12 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu)
> >  		gpu->ubwc_config.macrotile_mode = 1;
> >  	}
> >  
> > +	if (adreno_is_a663(gpu)) {
> > +		gpu->ubwc_config.highest_bank_bit = 13;
> > +		gpu->ubwc_config.ubwc_swizzle = 0x4;
> > +		gpu->ubwc_config.macrotile_mode = 1;
> 
> If this looks like A660 / A690, shouldn't the driver also enable .amsbc,
> .rgb565_predicator and .uavflagprd_inv?

You are right! Will fix in next patchset.

-Akhil.

> 
> > +	}
> > +
> >  	if (adreno_is_7c3(gpu)) {
> >  		gpu->ubwc_config.highest_bank_bit = 14;
> >  		gpu->ubwc_config.amsbc = 1;
> > @@ -1062,7 +1068,7 @@ static int hw_init(struct msm_gpu *gpu)
> >  	if (adreno_is_a690(adreno_gpu))
> >  		gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x90);
> >  	/* Set dualQ + disable afull for A660 GPU */
> > -	else if (adreno_is_a660(adreno_gpu))
> > +	else if (adreno_is_a660(adreno_gpu) || adreno_is_a663(adreno_gpu))
> >  		gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x66906);
> >  	else if (adreno_is_a7xx(adreno_gpu))
> >  		gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG,
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> > index cdb3f6e74d3e..f1196d66055c 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> > @@ -478,6 +478,37 @@ static void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> >  	msg->cnoc_cmds_data[1][0] =  0x60000001;
> >  }
> >  
> > +static void a663_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> > +{
> > +	/*
> > +	 * Send a single "off" entry just to get things running
> > +	 * TODO: bus scaling
> > +	 */
> > +	msg->bw_level_num = 1;
> > +
> > +	msg->ddr_cmds_num = 3;
> > +	msg->ddr_wait_bitmask = 0x07;
> > +
> > +	msg->ddr_cmds_addrs[0] = 0x50004;
> > +	msg->ddr_cmds_addrs[1] = 0x50000;
> > +	msg->ddr_cmds_addrs[2] = 0x500b4;
> > +
> > +	msg->ddr_cmds_data[0][0] =  0x40000000;
> > +	msg->ddr_cmds_data[0][1] =  0x40000000;
> > +	msg->ddr_cmds_data[0][2] =  0x40000000;
> > +
> > +	/*
> > +	 * These are the CX (CNOC) votes - these are used by the GMU but the
> > +	 * votes are known and fixed for the target
> > +	 */
> > +	msg->cnoc_cmds_num = 1;
> > +	msg->cnoc_wait_bitmask = 0x01;
> > +
> > +	msg->cnoc_cmds_addrs[0] = 0x50058;
> > +	msg->cnoc_cmds_data[0][0] =  0x40000000;
> > +	msg->cnoc_cmds_data[1][0] =  0x60000001;
> > +}
> > +
> >  static void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> >  {
> >  	/*
> > @@ -646,6 +677,8 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu)
> >  		adreno_7c3_build_bw_table(&msg);
> >  	else if (adreno_is_a660(adreno_gpu))
> >  		a660_build_bw_table(&msg);
> > +	else if (adreno_is_a663(adreno_gpu))
> > +		a663_build_bw_table(&msg);
> >  	else if (adreno_is_a690(adreno_gpu))
> >  		a690_build_bw_table(&msg);
> >  	else if (adreno_is_a730(adreno_gpu))
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > index 58d7e7915c57..10f8f25d8826 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > @@ -455,6 +455,11 @@ static inline int adreno_is_a680(const struct adreno_gpu *gpu)
> >  	return adreno_is_revn(gpu, 680);
> >  }
> >  
> > +static inline int adreno_is_a663(const struct adreno_gpu *gpu)
> > +{
> > +	return gpu->info->chip_ids[0] == 0x06060300;
> > +}
> > +
> >  static inline int adreno_is_a690(const struct adreno_gpu *gpu)
> >  {
> >  	return gpu->info->chip_ids[0] == 0x06090000;
> > 
> > -- 
> > 2.45.2
> > 
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH 1/3] drm/msm/a6xx: Add support for A663

[Akhil P Oommen]

On Wed, Sep 18, 2024 at 06:51:50PM +0100, Connor Abbott wrote:
> On Tue, Sep 17, 2024 at 9:39 PM Akhil P Oommen <quic_akhilpo@quicinc.com> wrote:
> >
> > From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> >
> > Add support for Adreno 663 found on sa8775p based platforms.
> >
> > Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> > ---
> >  drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 19 ++++++++++++++++++
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c     |  8 +++++++-
> >  drivers/gpu/drm/msm/adreno/a6xx_hfi.c     | 33 +++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.h   |  5 +++++
> >  4 files changed, 64 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> > index 0312b6ee0356..8d8d0d7630f0 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
> > @@ -972,6 +972,25 @@ static const struct adreno_info a6xx_gpus[] = {
> >                         .prim_fifo_threshold = 0x00300200,
> >                 },
> >                 .address_space_size = SZ_16G,
> > +       }, {
> > +               .chip_ids = ADRENO_CHIP_IDS(0x06060300),
> > +               .family = ADRENO_6XX_GEN4,
> > +               .fw = {
> > +                       [ADRENO_FW_SQE] = "a660_sqe.fw",
> > +                       [ADRENO_FW_GMU] = "a663_gmu.bin",
> > +               },
> > +               .gmem = SZ_1M + SZ_512K,
> > +               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > +               .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > +                       ADRENO_QUIRK_HAS_HW_APRIV,
> > +               .init = a6xx_gpu_init,
> > +               .a6xx = &(const struct a6xx_info) {
> > +                       .hwcg = a690_hwcg,
> > +                       .protect = &a660_protect,
> > +                       .gmu_cgc_mode = 0x00020200,
> > +                       .prim_fifo_threshold = 0x00300200,
> > +               },
> > +               .address_space_size = SZ_16G,
> >         }, {
> >                 .chip_ids = ADRENO_CHIP_IDS(0x06030500),
> >                 .family = ADRENO_6XX_GEN4,
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > index 06cab2c6fd66..e317780caeae 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > @@ -541,6 +541,12 @@ static void a6xx_calc_ubwc_config(struct adreno_gpu *gpu)
> >                 gpu->ubwc_config.macrotile_mode = 1;
> >         }
> >
> > +       if (adreno_is_a663(gpu)) {
> > +               gpu->ubwc_config.highest_bank_bit = 13;
> > +               gpu->ubwc_config.ubwc_swizzle = 0x4;
> 
> It's already been mentioned in the Mesa MR, but since this is the
> first GPU with level2_swizzling_dis set, the relevant vulkan CTS tests
> need to be tested with
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26578
> rebased on your Mesa enablement patch.

Will check this. But I prefer to not block DRM side patches on this.

-Akhil.

> 
> > +               gpu->ubwc_config.macrotile_mode = 1;
> > +       }
> > +
> >         if (adreno_is_7c3(gpu)) {
> >                 gpu->ubwc_config.highest_bank_bit = 14;
> >                 gpu->ubwc_config.amsbc = 1;
> > @@ -1062,7 +1068,7 @@ static int hw_init(struct msm_gpu *gpu)
> >         if (adreno_is_a690(adreno_gpu))
> >                 gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x90);
> >         /* Set dualQ + disable afull for A660 GPU */
> > -       else if (adreno_is_a660(adreno_gpu))
> > +       else if (adreno_is_a660(adreno_gpu) || adreno_is_a663(adreno_gpu))
> >                 gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x66906);
> >         else if (adreno_is_a7xx(adreno_gpu))
> >                 gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG,
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> > index cdb3f6e74d3e..f1196d66055c 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> > @@ -478,6 +478,37 @@ static void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> >         msg->cnoc_cmds_data[1][0] =  0x60000001;
> >  }
> >
> > +static void a663_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> > +{
> > +       /*
> > +        * Send a single "off" entry just to get things running
> > +        * TODO: bus scaling
> > +        */
> > +       msg->bw_level_num = 1;
> > +
> > +       msg->ddr_cmds_num = 3;
> > +       msg->ddr_wait_bitmask = 0x07;
> > +
> > +       msg->ddr_cmds_addrs[0] = 0x50004;
> > +       msg->ddr_cmds_addrs[1] = 0x50000;
> > +       msg->ddr_cmds_addrs[2] = 0x500b4;
> > +
> > +       msg->ddr_cmds_data[0][0] =  0x40000000;
> > +       msg->ddr_cmds_data[0][1] =  0x40000000;
> > +       msg->ddr_cmds_data[0][2] =  0x40000000;
> > +
> > +       /*
> > +        * These are the CX (CNOC) votes - these are used by the GMU but the
> > +        * votes are known and fixed for the target
> > +        */
> > +       msg->cnoc_cmds_num = 1;
> > +       msg->cnoc_wait_bitmask = 0x01;
> > +
> > +       msg->cnoc_cmds_addrs[0] = 0x50058;
> > +       msg->cnoc_cmds_data[0][0] =  0x40000000;
> > +       msg->cnoc_cmds_data[1][0] =  0x60000001;
> > +}
> > +
> >  static void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> >  {
> >         /*
> > @@ -646,6 +677,8 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu)
> >                 adreno_7c3_build_bw_table(&msg);
> >         else if (adreno_is_a660(adreno_gpu))
> >                 a660_build_bw_table(&msg);
> > +       else if (adreno_is_a663(adreno_gpu))
> > +               a663_build_bw_table(&msg);
> >         else if (adreno_is_a690(adreno_gpu))
> >                 a690_build_bw_table(&msg);
> >         else if (adreno_is_a730(adreno_gpu))
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > index 58d7e7915c57..10f8f25d8826 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > @@ -455,6 +455,11 @@ static inline int adreno_is_a680(const struct adreno_gpu *gpu)
> >         return adreno_is_revn(gpu, 680);
> >  }
> >
> > +static inline int adreno_is_a663(const struct adreno_gpu *gpu)
> > +{
> > +       return gpu->info->chip_ids[0] == 0x06060300;
> > +}
> > +
> >  static inline int adreno_is_a690(const struct adreno_gpu *gpu)
> >  {
> >         return gpu->info->chip_ids[0] == 0x06090000;
> >
> > --
> > 2.45.2
> >

Re: [PATCH 0/3] DRM/MSM: Support for Adreno 663 GPU

[Akhil P Oommen]

On Wed, Sep 18, 2024 at 12:34:32AM +0300, Dmitry Baryshkov wrote:
> On Wed, Sep 18, 2024 at 02:08:40AM GMT, Akhil P Oommen wrote:
> > This series adds support for Adreno 663 gpu found in SA8775P chipsets.
> > The closest gpu which is currently supported in drm-msm is A660.
> > Following are the major differences with that:
> > 	1. gmu/zap firmwares
> > 	2. Recommended to disable Level2 swizzling
> > 
> > Verified kmscube with the below Mesa change [1]. This series is rebased
> > on top of msm-next.
> 
> Is there a chance of you sharing Vulkan CTS results?

No. As of now there are no plans to run CTS.

-Akhil

> 
> > 
> > Patch (1) & (2) for Rob Clark and Patch (3) for Bjorn
> > 
> > [0] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/31211
> > 
> > To: Rob Clark <robdclark@gmail.com>
> > To: Sean Paul <sean@poorly.run>
> > To: Konrad Dybcio <konrad.dybcio@linaro.org>
> > To: Abhinav Kumar <quic_abhinavk@quicinc.com>
> > To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > To: Marijn Suijten <marijn.suijten@somainline.org>
> > To: David Airlie <airlied@gmail.com>
> > To: Daniel Vetter <daniel@ffwll.ch>
> > To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > To: Maxime Ripard <mripard@kernel.org>
> > To: Thomas Zimmermann <tzimmermann@suse.de>
> > To: Rob Herring <robh@kernel.org>
> > To: Krzysztof Kozlowski <krzk+dt@kernel.org>
> > To: Conor Dooley <conor+dt@kernel.org>
> > To: Bjorn Andersson <andersson@kernel.org>
> > Cc: linux-arm-msm@vger.kernel.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: freedreno@lists.freedesktop.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > 
> > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> > ---
> > Puranam V G Tejaswi (3):
> >       drm/msm/a6xx: Add support for A663
> >       dt-bindings: display/msm/gmu: Add Adreno 663 GMU
> >       arm64: dts: qcom: sa8775p: Add gpu and gmu nodes
> > 
> >  .../devicetree/bindings/display/msm/gmu.yaml       |  1 +
> >  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi         |  8 +++
> >  arch/arm64/boot/dts/qcom/sa8775p.dtsi              | 75 ++++++++++++++++++++++
> >  drivers/gpu/drm/msm/adreno/a6xx_catalog.c          | 19 ++++++
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c              |  8 ++-
> >  drivers/gpu/drm/msm/adreno/a6xx_hfi.c              | 33 ++++++++++
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.h            |  5 ++
> >  7 files changed, 148 insertions(+), 1 deletion(-)
> > ---
> > base-commit: 15302579373ed2c8ada629e9e7bcf9569393a48d
> > change-id: 20240917-a663-gpu-support-b1475c828606
> > 
> > Best regards,
> > -- 
> > Akhil P Oommen <quic_akhilpo@quicinc.com>
> > 
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH 3/3] arm64: dts: qcom: sa8775p: Add gpu and gmu nodes

[Akhil P Oommen]

On Wed, Sep 18, 2024 at 12:27:03AM +0300, Dmitry Baryshkov wrote:
> On Wed, Sep 18, 2024 at 02:08:43AM GMT, Akhil P Oommen wrote:
> > From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> > 
> > Add gpu and gmu nodes for sa8775p based platforms.
> 
> Which platforms? The commit adds nodes to the SoC and the single RIDE
> platform.
> 
> > 
> > Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi |  8 ++++
> >  arch/arm64/boot/dts/qcom/sa8775p.dtsi      | 75 ++++++++++++++++++++++++++++++
> >  2 files changed, 83 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > index 2a6170623ea9..a01e6675c4bb 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > @@ -407,6 +407,14 @@ queue3 {
> >  	};
> >  };
> >  
> > +&gpu {
> > +	status = "okay";
> > +
> > +	zap-shader {
> 
> It's easier to add gpu_zap_shader_link label in the DTSI file and then
> reference it instead of using the subnode again.
> 
> > +		firmware-name = "qcom/sa8775p/a663_zap.mbn";
> > +	};
> > +};
> 
> Separate patch, please.
> 
> > +
> >  &i2c11 {
> >  	clock-frequency = <400000>;
> >  	pinctrl-0 = <&qup_i2c11_default>;
> > diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > index 23f1b2e5e624..12c79135a303 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > @@ -2824,6 +2824,81 @@ tcsr_mutex: hwlock@1f40000 {
> >  			#hwlock-cells = <1>;
> >  		};
> >  
> > +		gpu: gpu@3d00000 {
> > +			compatible = "qcom,adreno-663.0", "qcom,adreno";
> > +			reg = <0 0x03d00000 0 0x40000>,
> > +			      <0 0x03d9e000 0 0x1000>,
> > +			      <0 0x03d61000 0 0x800>;
> 
> I think it's suggested to use 0x0 now
> 
> > +			reg-names = "kgsl_3d0_reg_memory",
> > +				    "cx_mem",
> > +				    "cx_dbgc";
> > +			interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> > +			iommus = <&adreno_smmu 0 0xc00>,
> > +				 <&adreno_smmu 1 0xc00>;
> > +			operating-points-v2 = <&gpu_opp_table>;
> > +			qcom,gmu = <&gmu>;
> > +			interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;
> 
> QCOM_ICC_TAG_ALWAYS instead of 0
> 
> > +			interconnect-names = "gfx-mem";
> > +			#cooling-cells = <2>;
> 
> No speed bins?

Thanks for the review. Agree on all comments.

Speedbins were missed because we are sharing these changes early in the
developement cycle, sort of like what we did for chromeos develeopment.
Will try to pick it up in the next patchset.

-Akhil

> 
> > +
> > +			status = "disabled";
> > +
> > +			zap-shader {
> 
> gpu_zap_shader: zap-shader
> 
> > +				memory-region = <&pil_gpu_mem>;
> > +			};
> > +
> > +			gpu_opp_table: opp-table {
> > +				compatible = "operating-points-v2";
> > +
> > +				opp-405000000 {
> 
> Just a single freq?
> 
> > +					opp-hz = /bits/ 64 <405000000>;
> > +					opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> > +					opp-peak-kBps = <8368000>;
> > +				};
> > +
> 
> Drop the empty line, please.
> 
> > +			};
> > +		};
> > +
> > +		gmu: gmu@3d6a000 {
> > +			compatible = "qcom,adreno-gmu-663.0", "qcom,adreno-gmu";
> > +			reg = <0 0x03d6a000 0 0x34000>,
> > +				<0 0x3de0000 0 0x10000>,
> > +				<0 0x0b290000 0 0x10000>;
> 
> Wrong indentation, please align to the angle bracket.
> Also I think it's suggested to use 0x0 now
> 
> > +			reg-names = "gmu", "rscc", "gmu_pdc";
> > +			interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
> > +					<GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
> 
> And here
> 
> > +			interrupt-names = "hfi", "gmu";
> > +			clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
> > +				 <&gpucc GPU_CC_CXO_CLK>,
> > +				 <&gcc GCC_DDRSS_GPU_AXI_CLK>,
> > +				 <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
> > +				 <&gpucc GPU_CC_AHB_CLK>,
> > +				 <&gpucc GPU_CC_HUB_CX_INT_CLK>,
> > +				 <&gpucc GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK>;
> > +			clock-names = "gmu",
> > +				      "cxo",
> > +				      "axi",
> > +				      "memnoc",
> > +				      "ahb",
> > +				      "hub",
> > +				      "smmu_vote";
> > +			power-domains = <&gpucc GPU_CC_CX_GDSC>,
> > +					<&gpucc GPU_CC_GX_GDSC>;
> > +			power-domain-names = "cx",
> > +					     "gx";
> > +			iommus = <&adreno_smmu 5 0xc00>;
> > +			operating-points-v2 = <&gmu_opp_table>;
> > +
> > +			gmu_opp_table: opp-table {
> > +				compatible = "operating-points-v2";
> > +
> > +				opp-200000000 {
> > +					opp-hz = /bits/ 64 <200000000>;
> > +					opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> > +				};
> > +			};
> > +		};
> > +
> >  		gpucc: clock-controller@3d90000 {
> >  			compatible = "qcom,sa8775p-gpucc";
> >  			reg = <0x0 0x03d90000 0x0 0xa000>;
> > 
> > -- 
> > 2.45.2
> > 
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH 3/3] arm64: dts: qcom: sa8775p: Add gpu and gmu nodes

[Dmitry Baryshkov]

On Mon, 23 Sept 2024 at 22:05, Akhil P Oommen <quic_akhilpo@quicinc.com> wrote:
>
> On Wed, Sep 18, 2024 at 12:27:03AM +0300, Dmitry Baryshkov wrote:
> > On Wed, Sep 18, 2024 at 02:08:43AM GMT, Akhil P Oommen wrote:
> > > From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> > >
> > > Add gpu and gmu nodes for sa8775p based platforms.
> >
> > Which platforms? The commit adds nodes to the SoC and the single RIDE
> > platform.
> >
> > >
> > > Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> > > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi |  8 ++++
> > >  arch/arm64/boot/dts/qcom/sa8775p.dtsi      | 75 ++++++++++++++++++++++++++++++
> > >  2 files changed, 83 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > > index 2a6170623ea9..a01e6675c4bb 100644
> > > --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > > @@ -407,6 +407,14 @@ queue3 {
> > >     };
> > >  };
> > >
> > > +&gpu {
> > > +   status = "okay";
> > > +
> > > +   zap-shader {
> >
> > It's easier to add gpu_zap_shader_link label in the DTSI file and then
> > reference it instead of using the subnode again.
> >
> > > +           firmware-name = "qcom/sa8775p/a663_zap.mbn";
> > > +   };
> > > +};
> >
> > Separate patch, please.
> >
> > > +
> > >  &i2c11 {
> > >     clock-frequency = <400000>;
> > >     pinctrl-0 = <&qup_i2c11_default>;
> > > diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > > index 23f1b2e5e624..12c79135a303 100644
> > > --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > > @@ -2824,6 +2824,81 @@ tcsr_mutex: hwlock@1f40000 {
> > >                     #hwlock-cells = <1>;
> > >             };
> > >
> > > +           gpu: gpu@3d00000 {
> > > +                   compatible = "qcom,adreno-663.0", "qcom,adreno";
> > > +                   reg = <0 0x03d00000 0 0x40000>,
> > > +                         <0 0x03d9e000 0 0x1000>,
> > > +                         <0 0x03d61000 0 0x800>;
> >
> > I think it's suggested to use 0x0 now
> >
> > > +                   reg-names = "kgsl_3d0_reg_memory",
> > > +                               "cx_mem",
> > > +                               "cx_dbgc";
> > > +                   interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> > > +                   iommus = <&adreno_smmu 0 0xc00>,
> > > +                            <&adreno_smmu 1 0xc00>;
> > > +                   operating-points-v2 = <&gpu_opp_table>;
> > > +                   qcom,gmu = <&gmu>;
> > > +                   interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;
> >
> > QCOM_ICC_TAG_ALWAYS instead of 0
> >
> > > +                   interconnect-names = "gfx-mem";
> > > +                   #cooling-cells = <2>;
> >
> > No speed bins?
>
> Thanks for the review. Agree on all comments.
>
> Speedbins were missed because we are sharing these changes early in the
> developement cycle, sort of like what we did for chromeos develeopment.
> Will try to pick it up in the next patchset.

Ack. If you mention this in the commit message, that would be great!

>
> -Akhil
>
> >
> > > +
> > > +                   status = "disabled";
> > > +
> > > +                   zap-shader {
> >
> > gpu_zap_shader: zap-shader
> >
> > > +                           memory-region = <&pil_gpu_mem>;
> > > +                   };
> > > +
> > > +                   gpu_opp_table: opp-table {
> > > +                           compatible = "operating-points-v2";
> > > +
> > > +                           opp-405000000 {
> >
> > Just a single freq?
> >
> > > +                                   opp-hz = /bits/ 64 <405000000>;
> > > +                                   opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> > > +                                   opp-peak-kBps = <8368000>;
> > > +                           };
> > > +
> >
> > Drop the empty line, please.
> >
> > > +                   };
> > > +           };
> > > +
> > > +           gmu: gmu@3d6a000 {
> > > +                   compatible = "qcom,adreno-gmu-663.0", "qcom,adreno-gmu";
> > > +                   reg = <0 0x03d6a000 0 0x34000>,
> > > +                           <0 0x3de0000 0 0x10000>,
> > > +                           <0 0x0b290000 0 0x10000>;
> >
> > Wrong indentation, please align to the angle bracket.
> > Also I think it's suggested to use 0x0 now
> >
> > > +                   reg-names = "gmu", "rscc", "gmu_pdc";
> > > +                   interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
> > > +                                   <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
> >
> > And here
> >
> > > +                   interrupt-names = "hfi", "gmu";
> > > +                   clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
> > > +                            <&gpucc GPU_CC_CXO_CLK>,
> > > +                            <&gcc GCC_DDRSS_GPU_AXI_CLK>,
> > > +                            <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
> > > +                            <&gpucc GPU_CC_AHB_CLK>,
> > > +                            <&gpucc GPU_CC_HUB_CX_INT_CLK>,
> > > +                            <&gpucc GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK>;
> > > +                   clock-names = "gmu",
> > > +                                 "cxo",
> > > +                                 "axi",
> > > +                                 "memnoc",
> > > +                                 "ahb",
> > > +                                 "hub",
> > > +                                 "smmu_vote";
> > > +                   power-domains = <&gpucc GPU_CC_CX_GDSC>,
> > > +                                   <&gpucc GPU_CC_GX_GDSC>;
> > > +                   power-domain-names = "cx",
> > > +                                        "gx";
> > > +                   iommus = <&adreno_smmu 5 0xc00>;
> > > +                   operating-points-v2 = <&gmu_opp_table>;
> > > +
> > > +                   gmu_opp_table: opp-table {
> > > +                           compatible = "operating-points-v2";
> > > +
> > > +                           opp-200000000 {
> > > +                                   opp-hz = /bits/ 64 <200000000>;
> > > +                                   opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> > > +                           };
> > > +                   };
> > > +           };
> > > +
> > >             gpucc: clock-controller@3d90000 {
> > >                     compatible = "qcom,sa8775p-gpucc";
> > >                     reg = <0x0 0x03d90000 0x0 0xa000>;
> > >
> > > --
> > > 2.45.2
> > >
> >
> > --
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry

Re: [PATCH 3/3] arm64: dts: qcom: sa8775p: Add gpu and gmu nodes

[Akhil P Oommen]

On Tue, Sep 24, 2024 at 08:14:17AM +0200, Dmitry Baryshkov wrote:
> On Mon, 23 Sept 2024 at 22:05, Akhil P Oommen <quic_akhilpo@quicinc.com> wrote:
> >
> > On Wed, Sep 18, 2024 at 12:27:03AM +0300, Dmitry Baryshkov wrote:
> > > On Wed, Sep 18, 2024 at 02:08:43AM GMT, Akhil P Oommen wrote:
> > > > From: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> > > >
> > > > Add gpu and gmu nodes for sa8775p based platforms.
> > >
> > > Which platforms? The commit adds nodes to the SoC and the single RIDE
> > > platform.
> > >
> > > >
> > > > Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@quicinc.com>
> > > > Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi |  8 ++++
> > > >  arch/arm64/boot/dts/qcom/sa8775p.dtsi      | 75 ++++++++++++++++++++++++++++++
> > > >  2 files changed, 83 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > > > index 2a6170623ea9..a01e6675c4bb 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > > > @@ -407,6 +407,14 @@ queue3 {
> > > >     };
> > > >  };
> > > >
> > > > +&gpu {
> > > > +   status = "okay";
> > > > +
> > > > +   zap-shader {
> > >
> > > It's easier to add gpu_zap_shader_link label in the DTSI file and then
> > > reference it instead of using the subnode again.
> > >
> > > > +           firmware-name = "qcom/sa8775p/a663_zap.mbn";
> > > > +   };
> > > > +};
> > >
> > > Separate patch, please.
> > >
> > > > +
> > > >  &i2c11 {
> > > >     clock-frequency = <400000>;
> > > >     pinctrl-0 = <&qup_i2c11_default>;
> > > > diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > > > index 23f1b2e5e624..12c79135a303 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > > > @@ -2824,6 +2824,81 @@ tcsr_mutex: hwlock@1f40000 {
> > > >                     #hwlock-cells = <1>;
> > > >             };
> > > >
> > > > +           gpu: gpu@3d00000 {
> > > > +                   compatible = "qcom,adreno-663.0", "qcom,adreno";
> > > > +                   reg = <0 0x03d00000 0 0x40000>,
> > > > +                         <0 0x03d9e000 0 0x1000>,
> > > > +                         <0 0x03d61000 0 0x800>;
> > >
> > > I think it's suggested to use 0x0 now
> > >
> > > > +                   reg-names = "kgsl_3d0_reg_memory",
> > > > +                               "cx_mem",
> > > > +                               "cx_dbgc";
> > > > +                   interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                   iommus = <&adreno_smmu 0 0xc00>,
> > > > +                            <&adreno_smmu 1 0xc00>;
> > > > +                   operating-points-v2 = <&gpu_opp_table>;
> > > > +                   qcom,gmu = <&gmu>;
> > > > +                   interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;
> > >
> > > QCOM_ICC_TAG_ALWAYS instead of 0
> > >
> > > > +                   interconnect-names = "gfx-mem";
> > > > +                   #cooling-cells = <2>;
> > >
> > > No speed bins?
> >
> > Thanks for the review. Agree on all comments.
> >
> > Speedbins were missed because we are sharing these changes early in the
> > developement cycle, sort of like what we did for chromeos develeopment.
> > Will try to pick it up in the next patchset.
> 
> Ack. If you mention this in the commit message, that would be great!

It looks like all SKUs have the same GPU fmax. Still I am checking with our
product team about the need for SKU detection. But that discussion will
probably take some time to close. I will post a separate series based on
its outcome. I am sending out v2 revision right away.

-Akhil
> 
> >
> > -Akhil
> >
> > >
> > > > +
> > > > +                   status = "disabled";
> > > > +
> > > > +                   zap-shader {
> > >
> > > gpu_zap_shader: zap-shader
> > >
> > > > +                           memory-region = <&pil_gpu_mem>;
> > > > +                   };
> > > > +
> > > > +                   gpu_opp_table: opp-table {
> > > > +                           compatible = "operating-points-v2";
> > > > +
> > > > +                           opp-405000000 {
> > >
> > > Just a single freq?
> > >
> > > > +                                   opp-hz = /bits/ 64 <405000000>;
> > > > +                                   opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> > > > +                                   opp-peak-kBps = <8368000>;
> > > > +                           };
> > > > +
> > >
> > > Drop the empty line, please.
> > >
> > > > +                   };
> > > > +           };
> > > > +
> > > > +           gmu: gmu@3d6a000 {
> > > > +                   compatible = "qcom,adreno-gmu-663.0", "qcom,adreno-gmu";
> > > > +                   reg = <0 0x03d6a000 0 0x34000>,
> > > > +                           <0 0x3de0000 0 0x10000>,
> > > > +                           <0 0x0b290000 0 0x10000>;
> > >
> > > Wrong indentation, please align to the angle bracket.
> > > Also I think it's suggested to use 0x0 now
> > >
> > > > +                   reg-names = "gmu", "rscc", "gmu_pdc";
> > > > +                   interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                                   <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
> > >
> > > And here
> > >
> > > > +                   interrupt-names = "hfi", "gmu";
> > > > +                   clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
> > > > +                            <&gpucc GPU_CC_CXO_CLK>,
> > > > +                            <&gcc GCC_DDRSS_GPU_AXI_CLK>,
> > > > +                            <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
> > > > +                            <&gpucc GPU_CC_AHB_CLK>,
> > > > +                            <&gpucc GPU_CC_HUB_CX_INT_CLK>,
> > > > +                            <&gpucc GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK>;
> > > > +                   clock-names = "gmu",
> > > > +                                 "cxo",
> > > > +                                 "axi",
> > > > +                                 "memnoc",
> > > > +                                 "ahb",
> > > > +                                 "hub",
> > > > +                                 "smmu_vote";
> > > > +                   power-domains = <&gpucc GPU_CC_CX_GDSC>,
> > > > +                                   <&gpucc GPU_CC_GX_GDSC>;
> > > > +                   power-domain-names = "cx",
> > > > +                                        "gx";
> > > > +                   iommus = <&adreno_smmu 5 0xc00>;
> > > > +                   operating-points-v2 = <&gmu_opp_table>;
> > > > +
> > > > +                   gmu_opp_table: opp-table {
> > > > +                           compatible = "operating-points-v2";
> > > > +
> > > > +                           opp-200000000 {
> > > > +                                   opp-hz = /bits/ 64 <200000000>;
> > > > +                                   opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> > > > +                           };
> > > > +                   };
> > > > +           };
> > > > +
> > > >             gpucc: clock-controller@3d90000 {
> > > >                     compatible = "qcom,sa8775p-gpucc";
> > > >                     reg = <0x0 0x03d90000 0x0 0xa000>;
> > > >
> > > > --
> > > > 2.45.2
> > > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
> 
> 
> 
> -- 
> With best wishes
> Dmitry