[PATCH 10/16] drm/msm/a6xx: Fix gpu init from secure world

Akhil P Oommen posted 16 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH 10/16] drm/msm/a6xx: Fix gpu init from secure world
Posted by Akhil P Oommen 1 week, 5 days ago
A7XX_GEN2 and newer GPUs requires initialization of few configurations
related to features/power from secure world. The SCM call to do this
should be triggered after GDSC and clocks are enabled. So, keep this
sequence to a6xx_gmu_resume instead of the probe.

Fixes: 14b27d5df3ea ("drm/msm/a7xx: Initialize a750 "software fuse"")
Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 58 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 59 -----------------------------------
 2 files changed, 58 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index cd6609bb66fe..61af3b212ba4 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -3,6 +3,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/clk.h>
+#include <linux/firmware/qcom/qcom_scm.h>
 #include <linux/interconnect.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
@@ -1174,6 +1175,59 @@ static void a6xx_gmu_set_initial_bw(struct msm_gpu *gpu, struct a6xx_gmu *gmu)
 	dev_pm_opp_put(gpu_opp);
 }
 
+static int a6xx_gmu_secure_init(struct a6xx_gpu *a6xx_gpu)
+{
+	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
+	struct msm_gpu *gpu = &adreno_gpu->base;
+	u32 fuse_val;
+	int ret;
+
+	if (adreno_is_a750(adreno_gpu) || adreno_is_a8xx(adreno_gpu)) {
+		/*
+		 * Assume that if qcom scm isn't available, that whatever
+		 * replacement allows writing the fuse register ourselves.
+		 * Users of alternative firmware need to make sure this
+		 * register is writeable or indicate that it's not somehow.
+		 * Print a warning because if you mess this up you're about to
+		 * crash horribly.
+		 */
+		if (!qcom_scm_is_available()) {
+			dev_warn_once(gpu->dev->dev,
+				"SCM is not available, poking fuse register\n");
+			a6xx_llc_write(a6xx_gpu, REG_A7XX_CX_MISC_SW_FUSE_VALUE,
+				A7XX_CX_MISC_SW_FUSE_VALUE_RAYTRACING |
+				A7XX_CX_MISC_SW_FUSE_VALUE_FASTBLEND |
+				A7XX_CX_MISC_SW_FUSE_VALUE_LPAC);
+			adreno_gpu->has_ray_tracing = true;
+			return 0;
+		}
+
+		ret = qcom_scm_gpu_init_regs(QCOM_SCM_GPU_ALWAYS_EN_REQ |
+					     QCOM_SCM_GPU_TSENSE_EN_REQ);
+		if (ret) {
+			dev_warn_once(gpu->dev->dev,
+				"SCM  call failed\n");
+			return ret;
+		}
+
+		/*
+		 * On A7XX_GEN3 and newer, raytracing may be disabled by the
+		 * firmware, find out whether that's the case. The scm call
+		 * above sets the fuse register.
+		 */
+		fuse_val = a6xx_llc_read(a6xx_gpu,
+					 REG_A7XX_CX_MISC_SW_FUSE_VALUE);
+		adreno_gpu->has_ray_tracing =
+			!!(fuse_val & A7XX_CX_MISC_SW_FUSE_VALUE_RAYTRACING);
+	} else if (adreno_is_a740(adreno_gpu)) {
+		/* Raytracing is always enabled on a740 */
+		adreno_gpu->has_ray_tracing = true;
+	}
+
+	return 0;
+}
+
+
 int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 {
 	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
@@ -1208,6 +1262,10 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 		return ret;
 	}
 
+	ret = a6xx_gmu_secure_init(a6xx_gpu);
+	if (ret)
+		return ret;
+
 	/* Read the slice info on A8x GPUs */
 	a8xx_gpu_get_slice_info(gpu);
 
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index cdecd91094c6..cbc803d90673 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -10,7 +10,6 @@
 
 #include <linux/bitfield.h>
 #include <linux/devfreq.h>
-#include <linux/firmware/qcom/qcom_scm.h>
 #include <linux/pm_domain.h>
 #include <linux/soc/qcom/llcc-qcom.h>
 
@@ -2158,56 +2157,6 @@ static void a6xx_llc_slices_init(struct platform_device *pdev,
 		a6xx_gpu->llc_mmio = ERR_PTR(-EINVAL);
 }
 
-static int a7xx_cx_mem_init(struct a6xx_gpu *a6xx_gpu)
-{
-	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
-	struct msm_gpu *gpu = &adreno_gpu->base;
-	u32 fuse_val;
-	int ret;
-
-	if (adreno_is_a750(adreno_gpu) || adreno_is_a8xx(adreno_gpu)) {
-		/*
-		 * Assume that if qcom scm isn't available, that whatever
-		 * replacement allows writing the fuse register ourselves.
-		 * Users of alternative firmware need to make sure this
-		 * register is writeable or indicate that it's not somehow.
-		 * Print a warning because if you mess this up you're about to
-		 * crash horribly.
-		 */
-		if (!qcom_scm_is_available()) {
-			dev_warn_once(gpu->dev->dev,
-				"SCM is not available, poking fuse register\n");
-			a6xx_llc_write(a6xx_gpu, REG_A7XX_CX_MISC_SW_FUSE_VALUE,
-				A7XX_CX_MISC_SW_FUSE_VALUE_RAYTRACING |
-				A7XX_CX_MISC_SW_FUSE_VALUE_FASTBLEND |
-				A7XX_CX_MISC_SW_FUSE_VALUE_LPAC);
-			adreno_gpu->has_ray_tracing = true;
-			return 0;
-		}
-
-		ret = qcom_scm_gpu_init_regs(QCOM_SCM_GPU_ALWAYS_EN_REQ |
-					     QCOM_SCM_GPU_TSENSE_EN_REQ);
-		if (ret)
-			return ret;
-
-		/*
-		 * On A7XX_GEN3 and newer, raytracing may be disabled by the
-		 * firmware, find out whether that's the case. The scm call
-		 * above sets the fuse register.
-		 */
-		fuse_val = a6xx_llc_read(a6xx_gpu,
-					 REG_A7XX_CX_MISC_SW_FUSE_VALUE);
-		adreno_gpu->has_ray_tracing =
-			!!(fuse_val & A7XX_CX_MISC_SW_FUSE_VALUE_RAYTRACING);
-	} else if (adreno_is_a740(adreno_gpu)) {
-		/* Raytracing is always enabled on a740 */
-		adreno_gpu->has_ray_tracing = true;
-	}
-
-	return 0;
-}
-
-
 #define GBIF_CLIENT_HALT_MASK		BIT(0)
 #define GBIF_ARB_HALT_MASK		BIT(1)
 #define VBIF_XIN_HALT_CTRL0_MASK	GENMASK(3, 0)
@@ -2711,14 +2660,6 @@ static struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
 		return ERR_PTR(ret);
 	}
 
-	if (adreno_is_a7xx(adreno_gpu) || adreno_is_a8xx(adreno_gpu)) {
-		ret = a7xx_cx_mem_init(a6xx_gpu);
-		if (ret) {
-			a6xx_destroy(&(a6xx_gpu->base.base));
-			return ERR_PTR(ret);
-		}
-	}
-
 	adreno_gpu->uche_trap_base = 0x1fffffffff000ull;
 
 	msm_mmu_set_fault_handler(to_msm_vm(gpu->vm)->mmu, gpu,

-- 
2.51.0
Re: [PATCH 10/16] drm/msm/a6xx: Fix gpu init from secure world
Posted by Konrad Dybcio 1 week, 5 days ago
On 3/23/26 9:12 PM, Akhil P Oommen wrote:
> A7XX_GEN2 and newer GPUs requires initialization of few configurations
> related to features/power from secure world. The SCM call to do this
> should be triggered after GDSC and clocks are enabled. So, keep this
> sequence to a6xx_gmu_resume instead of the probe.
> 
> Fixes: 14b27d5df3ea ("drm/msm/a7xx: Initialize a750 "software fuse"")
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---

This now makes this happen on *every GMU resume*, which sounds
undesirable (kgsl does this in gen7_gmu_first_boot)

Similarly, we call a8xx_gpu_get_slice_info() on *every resume*

Konrad
Re: [PATCH 10/16] drm/msm/a6xx: Fix gpu init from secure world
Posted by Akhil P Oommen 1 week, 2 days ago
On 3/24/2026 3:37 PM, Konrad Dybcio wrote:
> On 3/23/26 9:12 PM, Akhil P Oommen wrote:
>> A7XX_GEN2 and newer GPUs requires initialization of few configurations
>> related to features/power from secure world. The SCM call to do this
>> should be triggered after GDSC and clocks are enabled. So, keep this
>> sequence to a6xx_gmu_resume instead of the probe.
>>
>> Fixes: 14b27d5df3ea ("drm/msm/a7xx: Initialize a750 "software fuse"")
>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>> ---
> 
> This now makes this happen on *every GMU resume*, which sounds
> undesirable (kgsl does this in gen7_gmu_first_boot)

I had this in my TODO list, but forgot to update. Will fix.

> 
> Similarly, we call a8xx_gpu_get_slice_info() on *every resume*

No. We check then slice_mask and return early.

-Akhil.

> 
> Konrad
Re: [PATCH 10/16] drm/msm/a6xx: Fix gpu init from secure world
Posted by Konrad Dybcio 1 week, 2 days ago
On 3/26/26 9:12 PM, Akhil P Oommen wrote:
> On 3/24/2026 3:37 PM, Konrad Dybcio wrote:
>> On 3/23/26 9:12 PM, Akhil P Oommen wrote:
>>> A7XX_GEN2 and newer GPUs requires initialization of few configurations
>>> related to features/power from secure world. The SCM call to do this
>>> should be triggered after GDSC and clocks are enabled. So, keep this
>>> sequence to a6xx_gmu_resume instead of the probe.
>>>
>>> Fixes: 14b27d5df3ea ("drm/msm/a7xx: Initialize a750 "software fuse"")
>>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>>> ---
>>
>> This now makes this happen on *every GMU resume*, which sounds
>> undesirable (kgsl does this in gen7_gmu_first_boot)
> 
> I had this in my TODO list, but forgot to update. Will fix.
> 
>>
>> Similarly, we call a8xx_gpu_get_slice_info() on *every resume*
> 
> No. We check then slice_mask and return early.

Right, but we still branch into it

I think the best solution would be to add something like
gmu_first_boot_setup() which would be tracked by something like that
bit you added in v2, where similar things could be moved

That said, this is just an optimization

Konrad
Re: [PATCH 10/16] drm/msm/a6xx: Fix gpu init from secure world
Posted by Akhil P Oommen 5 days, 20 hours ago
On 3/27/2026 4:53 PM, Konrad Dybcio wrote:
> On 3/26/26 9:12 PM, Akhil P Oommen wrote:
>> On 3/24/2026 3:37 PM, Konrad Dybcio wrote:
>>> On 3/23/26 9:12 PM, Akhil P Oommen wrote:
>>>> A7XX_GEN2 and newer GPUs requires initialization of few configurations
>>>> related to features/power from secure world. The SCM call to do this
>>>> should be triggered after GDSC and clocks are enabled. So, keep this
>>>> sequence to a6xx_gmu_resume instead of the probe.
>>>>
>>>> Fixes: 14b27d5df3ea ("drm/msm/a7xx: Initialize a750 "software fuse"")
>>>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>>>> ---
>>>
>>> This now makes this happen on *every GMU resume*, which sounds
>>> undesirable (kgsl does this in gen7_gmu_first_boot)
>>
>> I had this in my TODO list, but forgot to update. Will fix.
>>
>>>
>>> Similarly, we call a8xx_gpu_get_slice_info() on *every resume*
>>
>> No. We check then slice_mask and return early.
> 
> Right, but we still branch into it
> 
> I think the best solution would be to add something like
> gmu_first_boot_setup() which would be tracked by something like that
> bit you added in v2, where similar things could be moved
> 
> That said, this is just an optimization

Perhaps, a micro-optimization. ;)

-Akhil

> 
> Konrad