[PATCH RFT 0/5] drm/msm: Attach a driver to GMU

[PATCH RFT 0/5] drm/msm: Attach a driver to GMU

[Akhil P Oommen]

With the introduction of sync_state in the genpd framework, any consumer
device of GCC and GPUCC which is not bound to a driver will result in
bootup warnings like below:

[   24.362666] gcc-kaanapali 100000.clock-controller: sync_state() pending due to 3d37000.gmu
[   24.371210] gxclkctl-kaanapali 3d64000.clock-controller: sync_state() pending due to 3d37000.gmu
[   24.380268] gpucc-kaanapali 3d90000.clock-controller: sync_state() pending due to 3d37000.gmu

To silence these warnings and also to have a proper state in driver core,
attach a driver to the GMU and set it up as a component device for the drm
master device.

The first few patches are preparatory fixes and refactorings. And the
final patch introduces a driver for GMU devices.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
Akhil P Oommen (5):
      drm/msm/adreno/a6xx: Mark cxpd device_link as stateless
      drm/msm: Centralize the standalone drm device check for GPU
      drm/msm/adreno: Fix invalid drvdata typecast in adreno_remove()
      drm/msm: Always use component model for standalone GPU
      drm/msm: Attach a driver to the GMU

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c      | 76 ++++++++++++++++++++++++++++--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c      | 30 +++++-------
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h      |  3 --
 drivers/gpu/drm/msm/adreno/adreno_device.c | 27 +++++++----
 drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  3 +-
 drivers/gpu/drm/msm/msm_drv.c              | 49 ++++++++++++++-----
 drivers/gpu/drm/msm/msm_drv.h              |  8 ++--
 7 files changed, 143 insertions(+), 53 deletions(-)
---
base-commit: bb1459368dd795c43380057523f571d5eb0ddded
change-id: 20260511-gmu-sync-state-fix-bc6b2d809371

Best regards,
-- 
Akhil P Oommen <akhilpo@oss.qualcomm.com>

[PATCH RFT 1/5] drm/msm/adreno/a6xx: Mark cxpd device_link as stateless

[Akhil P Oommen]

Since the cxpd virtual device is a driverless device, it has no real
struct device_driver bound it the way a normal platform device does. So,
the managed lifecycle doesn't map cleanly onto it. Since the lifecycle of
this device_link is managed by the msm driver, pass DL_FLAG_STATELESS
together with the existing DL_FLAG_PM_RUNTIME flag.

This fix is required to avoid the below splat when a driver is attached to
the GMU:

[    9.624509] WARNING: drivers/base/core.c:1383 at dvice_links_driver_bound+0x178/0x330, CPU#6: (udev-worker)/354e
[    9.781167] pc : device_links_driver_bound+0x178/0x330
[    9.786456] lr : device_links_driver_bound+0x158/0x330
[    9.798622] sp : ffff8000827bb820
[    9.802042] x29: ffff8000827bb820 x28: ffff000808915800 x27: 0000000000000044
[    9.809573] x26: ffffb7caf3f983d0 x25: 0000000000000003 x24: ffffb7caf41645a0
[    9.816920] x23: ffffb7caf3f982d8 x22: ffff000801239cb8 x21: ffff000801239c10
[    9.816921] x20: ffff00080b3a8c00 x19: ffff000801239c98 x18: 00000000ffffffff
[    9.816922] x17: 736d5b2073706f5f x16: ffffb7caf1eb5940 x15: ffff8001027bb3b7
[    9.816922] x14: 0000000000000026 x13: ffff00080084c110 x12: 0000000000000000
[    9.816923] x11: ffff0008004a5828 x10: ffff0008004a56d0 x9 : ffffb7caf2227870
[    9.816924] x8 : ffff0008004a56f8 x7 : 0000000000000001 x6 : 0000000000001000ocket.
[    9.816925] x5 : 00000000001a7fbf x4 : ffff0008004a56e8 x3 : ffff000801238cc8
[    9.816926] x2 : ffffb7caf4164000 x1 : ffff000801239c10 x0 : 0000000000000000
[    9.816927] Call trace:
[    9.816927]  device_links_driver_bound+0x178/0x330 (P)
[    9.893492]  driver_bound+0x7c/0xd0
[    9.893493]  really_probe+0x208/0x2a8
[    9.893495]  __driver_probe_device+0x88/0x170
[    9.893496]  driver_probe_device+0x44/0x178
[    9.909639]  __driver_attach+0x9c/0x1b8
[    9.913592]  bus_for_each_dev+0x7c/0xe8System Message Bus Socket.
[    9.925839]  driver_attach+0x2c/0x40
[    9.929733]  bus_add_driver+0xec/0x218
[    9.933609]  driver_register+0x68/0x138
[    9.937555]  __platform_driver_register+0x2c/0x40
[    9.937556]  adreno_gmu_register+0x2c/0x40 [msm]
[    9.947141]  adreno_register+0x3c/0x50 [msm]
[    9.951544]  msm_drm_register+0x50/0x78 [msm]for the API...
[    9.962813]  do_one_initcall+0x4c/0x3e0
[    9.966975]  do_init_module+0x60/0x280
[    9.970849]  load_module+0x1c70/0x1fa0
[    9.974708]  init_module_from_file+0xdc/0x100
[    9.979188]  __arm64_sys_finit_module+0x1c0/0x2e0
[    9.984026]  invoke_syscall+0x5c/0x120
[    9.987882]  el0_svc_common.constprop.0+0xd0/0xf8
[    9.992715]  do_el0_svc+0x28/0x40
[    9.995243] qcom_q6v5_pas 6800000.remoteproc: Handover signaled, but it already happened
[    9.996128]  el0_svc+0x38/0x148
[   10.007672]  el0t_64_sync_handler+0xa0/0xe8
[   10.011975]  el0t_64_sync+0x198/0x1a0

Fixes: ead5d3e5eb37 ("drm/msm/a6xx: Vote for cx gdsc from gpu driver")
Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 1b44b9e21ad8..ec13b27feee7 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -2196,7 +2196,8 @@ int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 		goto err_mmio;
 	}
 
-	if (!device_link_add(gmu->dev, gmu->cxpd, DL_FLAG_PM_RUNTIME)) {
+	if (!device_link_add(gmu->dev, gmu->cxpd,
+			     DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS)) {
 		ret = -ENODEV;
 		goto detach_cxpd;
 	}
@@ -2377,7 +2378,8 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 		goto err_mmio;
 	}
 
-	link = device_link_add(gmu->dev, gmu->cxpd, DL_FLAG_PM_RUNTIME);
+	link = device_link_add(gmu->dev, gmu->cxpd,
+			       DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
 	if (!link) {
 		ret = -ENODEV;
 		goto detach_cxpd;

-- 
2.51.0

Re: [PATCH RFT 1/5] drm/msm/adreno/a6xx: Mark cxpd device_link as stateless

[Dmitry Baryshkov]

On Wed, May 13, 2026 at 02:22:56AM +0530, Akhil P Oommen wrote:
> Since the cxpd virtual device is a driverless device, it has no real
> struct device_driver bound it the way a normal platform device does. So,
> the managed lifecycle doesn't map cleanly onto it. Since the lifecycle of
> this device_link is managed by the msm driver, pass DL_FLAG_STATELESS
> together with the existing DL_FLAG_PM_RUNTIME flag.
> 
> This fix is required to avoid the below splat when a driver is attached to
> the GMU:
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Claude review: drm/msm/adreno/a6xx: Mark cxpd device_link as stateless

[Claude Code Review Bot]

Patch Review

**Verdict: Looks good.**

The cxpd virtual device has no real driver, so `device_links_driver_bound()` warns when the GMU driver binds and finds a managed (non-stateless) device_link pointing to a supplier without a driver. Adding `DL_FLAG_STATELESS` is the correct fix since the link's lifecycle is already managed manually by the MSM driver.

```c
+	if (!device_link_add(gmu->dev, gmu->cxpd,
+			     DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS)) {
```

The same fix is applied consistently to both `a6xx_gmu_wrapper_init` and `a6xx_gmu_init`. The Fixes tag referencing `ead5d3e5eb37` is appropriate.

Minor: The commit message explains the fix well, and the stack trace included makes the problem clear.

---

---
Generated by Claude Code Patch Reviewer

[PATCH RFT 2/5] drm/msm: Centralize the standalone drm device check for GPU

[Akhil P Oommen]

Replace msm_gpu_no_components() with msm_gpu_use_separate_drm_dev() and
move the platform-specific check for "amd,imageon" into the helper so the
policy is centralized and reused by both the core driver and adreno probe
path. No functional change intended.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/adreno_device.c |  3 +--
 drivers/gpu/drm/msm/msm_drv.c              | 11 +++++++----
 drivers/gpu/drm/msm/msm_drv.h              |  2 +-
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 4edfe80c5be7..3052f3e36de0 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -271,8 +271,7 @@ static const struct component_ops a3xx_ops = {
 
 static int adreno_probe(struct platform_device *pdev)
 {
-	if (of_device_is_compatible(pdev->dev.of_node, "amd,imageon") ||
-	    msm_gpu_no_components())
+	if (msm_gpu_use_separate_drm_dev(pdev))
 		return msm_gpu_probe(pdev, &a3xx_ops);
 
 	return component_add(&pdev->dev, &a3xx_ops);
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 195f40e331e5..b61deafd02c3 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -61,9 +61,12 @@ module_param(separate_gpu_kms, bool, 0400);
 DECLARE_FAULT_ATTR(fail_gem_alloc);
 DECLARE_FAULT_ATTR(fail_gem_iova);
 
-bool msm_gpu_no_components(void)
+bool msm_gpu_use_separate_drm_dev(struct platform_device *pdev)
 {
-	return separate_gpu_kms;
+	if (!pdev)
+		return separate_gpu_kms;
+
+	return of_device_is_compatible(pdev->dev.of_node, "amd,imageon") || separate_gpu_kms;
 }
 
 static int msm_drm_uninit(struct device *dev, const struct component_ops *gpu_ops)
@@ -1035,7 +1038,7 @@ static int add_gpu_components(struct device *dev,
 static int msm_drm_bind(struct device *dev)
 {
 	return msm_drm_init(dev,
-			    msm_gpu_no_components() ?
+			    msm_gpu_use_separate_drm_dev(NULL) ?
 				    &msm_kms_driver :
 				    &msm_driver,
 			    NULL);
@@ -1074,7 +1077,7 @@ int msm_drv_probe(struct device *master_dev,
 			return ret;
 	}
 
-	if (!msm_gpu_no_components()) {
+	if (!msm_gpu_use_separate_drm_dev(NULL)) {
 		ret = add_gpu_components(master_dev, &match);
 		if (ret)
 			return ret;
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 6d847d593f1a..6fcb696ceb7c 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -555,6 +555,6 @@ void msm_kms_shutdown(struct platform_device *pdev);
 
 bool msm_disp_drv_should_bind(struct device *dev, bool dpu_driver);
 
-bool msm_gpu_no_components(void);
+bool msm_gpu_use_separate_drm_dev(struct platform_device *pdev);
 
 #endif /* __MSM_DRV_H__ */

-- 
2.51.0

Re: [PATCH RFT 2/5] drm/msm: Centralize the standalone drm device check for GPU

[Dmitry Baryshkov]

On Wed, May 13, 2026 at 02:22:57AM +0530, Akhil P Oommen wrote:
> Replace msm_gpu_no_components() with msm_gpu_use_separate_drm_dev() and
> move the platform-specific check for "amd,imageon" into the helper so the
> policy is centralized and reused by both the core driver and adreno probe
> path. No functional change intended.
> 
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_device.c |  3 +--
>  drivers/gpu/drm/msm/msm_drv.c              | 11 +++++++----
>  drivers/gpu/drm/msm/msm_drv.h              |  2 +-
>  3 files changed, 9 insertions(+), 7 deletions(-)

For the next iteration please swap this and the next patches (yes,
introducing some code duplication). It's generally not recommended to
have fixes which depend on a non-fix patches.


-- 
With best wishes
Dmitry

Re: [PATCH RFT 2/5] drm/msm: Centralize the standalone drm device check for GPU

[Akhil P Oommen]

On 5/13/2026 11:11 AM, Dmitry Baryshkov wrote:
> On Wed, May 13, 2026 at 02:22:57AM +0530, Akhil P Oommen wrote:
>> Replace msm_gpu_no_components() with msm_gpu_use_separate_drm_dev() and
>> move the platform-specific check for "amd,imageon" into the helper so the
>> policy is centralized and reused by both the core driver and adreno probe
>> path. No functional change intended.
>>
>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>> ---
>>  drivers/gpu/drm/msm/adreno/adreno_device.c |  3 +--
>>  drivers/gpu/drm/msm/msm_drv.c              | 11 +++++++----
>>  drivers/gpu/drm/msm/msm_drv.h              |  2 +-
>>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> For the next iteration please swap this and the next patches (yes,
> introducing some code duplication). It's generally not recommended to
> have fixes which depend on a non-fix patches.

Ack.

-Akhil.

> 
> 

Claude review: drm/msm: Centralize the standalone drm device check for GPU

[Claude Code Review Bot]

Patch Review

**Verdict: Acceptable, but the NULL parameter convention is awkward.**

The rename from `msm_gpu_no_components()` to `msm_gpu_use_separate_drm_dev(pdev)` centralizes the `amd,imageon` compatibility check. However, the two call sites in `msm_drv.c` pass `NULL`:

```c
+			    msm_gpu_use_separate_drm_dev(NULL) ?
```
```c
+	if (!msm_gpu_use_separate_drm_dev(NULL)) {
```

And the function has a special case for NULL:

```c
+bool msm_gpu_use_separate_drm_dev(struct platform_device *pdev)
+{
+	if (!pdev)
+		return separate_gpu_kms;
+
+	return of_device_is_compatible(pdev->dev.of_node, "amd,imageon") || separate_gpu_kms;
+}
```

Passing `NULL` to skip the imageon check is a code smell. These callers don't have a pdev and really only care about `separate_gpu_kms`. Consider either keeping a no-argument helper for that case, or documenting why the NULL-with-different-semantics pattern is intentional. As-is, a reader has to read the function body to understand that `NULL` means "just check the module param."

---

---
Generated by Claude Code Patch Reviewer

[PATCH RFT 3/5] drm/msm/adreno: Fix invalid drvdata typecast in adreno_remove()

[Akhil P Oommen]

adreno_remove() decides between component_del() and msm_gpu_remove()
based on platform_get_drvdata(pdev), interpreted as a struct
msm_drm_private *. That dereference is wrong because msm_gpu_init()
overwrites the GPU pdev's drvdata with adreno_smmu_priv pointer.

Fix this by using msm_gpu_use_separate_drm_dev() instead of relying on
drvdata.

Fixes: 643515a9cd09 ("drm/msm: rework binding of Imageon GPUs")
Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 3052f3e36de0..66953e551d86 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -279,12 +279,10 @@ static int adreno_probe(struct platform_device *pdev)
 
 static void adreno_remove(struct platform_device *pdev)
 {
-	struct msm_drm_private *priv = platform_get_drvdata(pdev);
-
-	if (priv->kms_init)
-		component_del(&pdev->dev, &a3xx_ops);
-	else
+	if (msm_gpu_use_separate_drm_dev(pdev))
 		msm_gpu_remove(pdev, &a3xx_ops);
+	else
+		component_del(&pdev->dev, &a3xx_ops);
 }
 
 static void adreno_shutdown(struct platform_device *pdev)

-- 
2.51.0

Re: [PATCH RFT 3/5] drm/msm/adreno: Fix invalid drvdata typecast in adreno_remove()

[Dmitry Baryshkov]

On Wed, May 13, 2026 at 02:22:58AM +0530, Akhil P Oommen wrote:
> adreno_remove() decides between component_del() and msm_gpu_remove()
> based on platform_get_drvdata(pdev), interpreted as a struct
> msm_drm_private *. That dereference is wrong because msm_gpu_init()
> overwrites the GPU pdev's drvdata with adreno_smmu_priv pointer.
> 
> Fix this by using msm_gpu_use_separate_drm_dev() instead of relying on
> drvdata.
> 
> Fixes: 643515a9cd09 ("drm/msm: rework binding of Imageon GPUs")
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_device.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Claude review: drm/msm/adreno: Fix invalid drvdata typecast in adreno_remove()

[Claude Code Review Bot]

Patch Review

**Verdict: Good bugfix.**

The old code interpreted the GPU pdev's drvdata as `struct msm_drm_private *`:

```c
-	struct msm_drm_private *priv = platform_get_drvdata(pdev);
-
-	if (priv->kms_init)
-		component_del(&pdev->dev, &a3xx_ops);
-	else
-		msm_gpu_remove(pdev, &a3xx_ops);
```

But `msm_gpu.c:1072` shows `platform_set_drvdata(pdev, &gpu->adreno_smmu)` -- the drvdata is actually `adreno_smmu_priv`, not `msm_drm_private`. So `priv->kms_init` was reading garbage from an unrelated struct. The fix replaces this with `msm_gpu_use_separate_drm_dev(pdev)`, which doesn't depend on drvdata at all.

The Fixes tag referencing `643515a9cd09` is appropriate.

---

---
Generated by Claude Code Patch Reviewer

[PATCH RFT 4/5] drm/msm: Always use component model for standalone GPU

[Akhil P Oommen]

When separate_gpu_kms=1 (or the device is compatible with "amd,imageon"),
msm_gpu_no_components() evaluates true and adreno_probe() bypasses the
component framework by calling msm_gpu_probe() directly. In this case,
we skip creating components and directly bind the GPU.

That shortcut makes it impossible to add a second sibling on the GPU
master without introducing ad-hoc plumbing. To prepare for adding the
GMU as a peer component on this master, turn the GPU pdev into both a
component master and the sole component of itself in this path. A follow
up patch will introduce GMU as a component device.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 20 ++++++++++++-----
 drivers/gpu/drm/msm/msm_drv.c              | 35 +++++++++++++++++++++++++-----
 drivers/gpu/drm/msm/msm_drv.h              |  6 ++---
 3 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 66953e551d86..67686424f3a1 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -271,18 +271,26 @@ static const struct component_ops a3xx_ops = {
 
 static int adreno_probe(struct platform_device *pdev)
 {
-	if (msm_gpu_use_separate_drm_dev(pdev))
-		return msm_gpu_probe(pdev, &a3xx_ops);
+	int ret;
 
-	return component_add(&pdev->dev, &a3xx_ops);
+	if (msm_gpu_use_separate_drm_dev(pdev)) {
+		ret = msm_gpu_probe(pdev);
+		if (ret)
+			return ret;
+	}
+
+	ret = component_add(&pdev->dev, &a3xx_ops);
+	if (ret && msm_gpu_use_separate_drm_dev(pdev))
+		msm_gpu_remove(pdev);
+
+	return ret;
 }
 
 static void adreno_remove(struct platform_device *pdev)
 {
+	component_del(&pdev->dev, &a3xx_ops);
 	if (msm_gpu_use_separate_drm_dev(pdev))
-		msm_gpu_remove(pdev, &a3xx_ops);
-	else
-		component_del(&pdev->dev, &a3xx_ops);
+		msm_gpu_remove(pdev);
 }
 
 static void adreno_shutdown(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index b61deafd02c3..af5aa7ff6179 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1097,10 +1097,25 @@ int msm_drv_probe(struct device *master_dev,
 	return 0;
 }
 
-int msm_gpu_probe(struct platform_device *pdev,
-		  const struct component_ops *ops)
+static int msm_gpu_drm_bind(struct device *dev)
+{
+	return msm_drm_init(dev, &msm_gpu_driver, NULL);
+}
+
+static void msm_gpu_drm_unbind(struct device *dev)
+{
+	msm_drm_uninit(dev, NULL);
+}
+
+static const struct component_master_ops msm_gpu_drm_ops = {
+	.bind = msm_gpu_drm_bind,
+	.unbind = msm_gpu_drm_unbind,
+};
+
+int msm_gpu_probe(struct platform_device *pdev)
 {
 	struct msm_drm_private *priv;
+	struct component_match *match = NULL;
 	int ret;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -1116,13 +1131,21 @@ int msm_gpu_probe(struct platform_device *pdev,
 	if (ret)
 		return ret;
 
-	return msm_drm_init(&pdev->dev, &msm_gpu_driver, ops);
+	/*
+	 * The GPU pdev acts as both the component master and the sole
+	 * component (added by adreno_probe()). Future patches add the
+	 * GMU node as a second component on this same master.
+	 */
+	drm_of_component_match_add(&pdev->dev, &match,
+				   component_compare_of, pdev->dev.of_node);
+
+	return component_master_add_with_match(&pdev->dev, &msm_gpu_drm_ops,
+					       match);
 }
 
-void msm_gpu_remove(struct platform_device *pdev,
-		    const struct component_ops *ops)
+void msm_gpu_remove(struct platform_device *pdev)
 {
-	msm_drm_uninit(&pdev->dev, ops);
+	component_master_del(&pdev->dev, &msm_gpu_drm_ops);
 }
 
 static int __init msm_drm_register(void)
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 6fcb696ceb7c..6264ff27496f 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -544,10 +544,8 @@ extern const struct component_master_ops msm_drm_ops;
 int msm_kms_pm_prepare(struct device *dev);
 void msm_kms_pm_complete(struct device *dev);
 
-int msm_gpu_probe(struct platform_device *pdev,
-		  const struct component_ops *ops);
-void msm_gpu_remove(struct platform_device *pdev,
-		    const struct component_ops *ops);
+int msm_gpu_probe(struct platform_device *pdev);
+void msm_gpu_remove(struct platform_device *pdev);
 int msm_drv_probe(struct device *dev,
 	int (*kms_init)(struct drm_device *dev),
 	struct msm_kms *kms);

-- 
2.51.0

Re: [PATCH RFT 4/5] drm/msm: Always use component model for standalone GPU

[Dmitry Baryshkov]

On Wed, May 13, 2026 at 02:22:59AM +0530, Akhil P Oommen wrote:
> When separate_gpu_kms=1 (or the device is compatible with "amd,imageon"),
> msm_gpu_no_components() evaluates true and adreno_probe() bypasses the
> component framework by calling msm_gpu_probe() directly. In this case,
> we skip creating components and directly bind the GPU.
> 
> That shortcut makes it impossible to add a second sibling on the GPU
> master without introducing ad-hoc plumbing. To prepare for adding the
> GMU as a peer component on this master, turn the GPU pdev into both a
> component master and the sole component of itself in this path. A follow
> up patch will introduce GMU as a component device.
> 
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_device.c | 20 ++++++++++++-----
>  drivers/gpu/drm/msm/msm_drv.c              | 35 +++++++++++++++++++++++++-----
>  drivers/gpu/drm/msm/msm_drv.h              |  6 ++---
>  3 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 66953e551d86..67686424f3a1 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -271,18 +271,26 @@ static const struct component_ops a3xx_ops = {
>  
>  static int adreno_probe(struct platform_device *pdev)
>  {
> -	if (msm_gpu_use_separate_drm_dev(pdev))
> -		return msm_gpu_probe(pdev, &a3xx_ops);
> +	int ret;
>  
> -	return component_add(&pdev->dev, &a3xx_ops);
> +	if (msm_gpu_use_separate_drm_dev(pdev)) {
> +		ret = msm_gpu_probe(pdev);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = component_add(&pdev->dev, &a3xx_ops);
> +	if (ret && msm_gpu_use_separate_drm_dev(pdev))
> +		msm_gpu_remove(pdev);

What about making it more clear:

	if (!msm_gpu_use_separate_drm_dev(pdev))
		return component_add(&pdev->dev, &a3xx_ops);

	ret = msm_gpu_probe(pdev);
	if (ret)
		return ret;

	ret = component_add(&pdev->dev, &a3xx_ops);

	if (ret)
		msm_gpu_remove(pdev);

	return ret;

However with this patch in place, maybe it's easier to use the main
msm_drv_probe()? It would need some fixes to handle !kms case as the
GPU-only, but it looks very similar to your new functions.


> +
> +	return ret;
>  }
>  
>  static void adreno_remove(struct platform_device *pdev)
>  {
> +	component_del(&pdev->dev, &a3xx_ops);
>  	if (msm_gpu_use_separate_drm_dev(pdev))
> -		msm_gpu_remove(pdev, &a3xx_ops);
> -	else
> -		component_del(&pdev->dev, &a3xx_ops);
> +		msm_gpu_remove(pdev);
>  }
>  
>  static void adreno_shutdown(struct platform_device *pdev)
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index b61deafd02c3..af5aa7ff6179 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -1097,10 +1097,25 @@ int msm_drv_probe(struct device *master_dev,
>  	return 0;
>  }
>  
> -int msm_gpu_probe(struct platform_device *pdev,
> -		  const struct component_ops *ops)
> +static int msm_gpu_drm_bind(struct device *dev)
> +{
> +	return msm_drm_init(dev, &msm_gpu_driver, NULL);

With this patch in place, we can remove the ops argument from
msm_drm_init() and msm_drm_uninit().

> +}
> +
> +static void msm_gpu_drm_unbind(struct device *dev)
> +{
> +	msm_drm_uninit(dev, NULL);
> +}
> +
> +static const struct component_master_ops msm_gpu_drm_ops = {
> +	.bind = msm_gpu_drm_bind,
> +	.unbind = msm_gpu_drm_unbind,
> +};
> +
> +int msm_gpu_probe(struct platform_device *pdev)
>  {
>  	struct msm_drm_private *priv;
> +	struct component_match *match = NULL;
>  	int ret;
>  
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> @@ -1116,13 +1131,21 @@ int msm_gpu_probe(struct platform_device *pdev,
>  	if (ret)
>  		return ret;
>  
> -	return msm_drm_init(&pdev->dev, &msm_gpu_driver, ops);
> +	/*
> +	 * The GPU pdev acts as both the component master and the sole
> +	 * component (added by adreno_probe()). Future patches add the
> +	 * GMU node as a second component on this same master.
> +	 */
> +	drm_of_component_match_add(&pdev->dev, &match,
> +				   component_compare_of, pdev->dev.of_node);
> +
> +	return component_master_add_with_match(&pdev->dev, &msm_gpu_drm_ops,
> +					       match);
>  }
>  
> -void msm_gpu_remove(struct platform_device *pdev,
> -		    const struct component_ops *ops)
> +void msm_gpu_remove(struct platform_device *pdev)
>  {
> -	msm_drm_uninit(&pdev->dev, ops);
> +	component_master_del(&pdev->dev, &msm_gpu_drm_ops);
>  }
>  
>  static int __init msm_drm_register(void)
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 6fcb696ceb7c..6264ff27496f 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -544,10 +544,8 @@ extern const struct component_master_ops msm_drm_ops;
>  int msm_kms_pm_prepare(struct device *dev);
>  void msm_kms_pm_complete(struct device *dev);
>  
> -int msm_gpu_probe(struct platform_device *pdev,
> -		  const struct component_ops *ops);
> -void msm_gpu_remove(struct platform_device *pdev,
> -		    const struct component_ops *ops);
> +int msm_gpu_probe(struct platform_device *pdev);
> +void msm_gpu_remove(struct platform_device *pdev);
>  int msm_drv_probe(struct device *dev,
>  	int (*kms_init)(struct drm_device *dev),
>  	struct msm_kms *kms);
> 
> -- 
> 2.51.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH RFT 4/5] drm/msm: Always use component model for standalone GPU

[Akhil P Oommen]

On 5/13/2026 5:13 PM, Dmitry Baryshkov wrote:
> On Wed, May 13, 2026 at 02:22:59AM +0530, Akhil P Oommen wrote:
>> When separate_gpu_kms=1 (or the device is compatible with "amd,imageon"),
>> msm_gpu_no_components() evaluates true and adreno_probe() bypasses the
>> component framework by calling msm_gpu_probe() directly. In this case,
>> we skip creating components and directly bind the GPU.
>>
>> That shortcut makes it impossible to add a second sibling on the GPU
>> master without introducing ad-hoc plumbing. To prepare for adding the
>> GMU as a peer component on this master, turn the GPU pdev into both a
>> component master and the sole component of itself in this path. A follow
>> up patch will introduce GMU as a component device.
>>
>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>> ---
>>  drivers/gpu/drm/msm/adreno/adreno_device.c | 20 ++++++++++++-----
>>  drivers/gpu/drm/msm/msm_drv.c              | 35 +++++++++++++++++++++++++-----
>>  drivers/gpu/drm/msm/msm_drv.h              |  6 ++---
>>  3 files changed, 45 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> index 66953e551d86..67686424f3a1 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> @@ -271,18 +271,26 @@ static const struct component_ops a3xx_ops = {
>>  
>>  static int adreno_probe(struct platform_device *pdev)
>>  {
>> -	if (msm_gpu_use_separate_drm_dev(pdev))
>> -		return msm_gpu_probe(pdev, &a3xx_ops);
>> +	int ret;
>>  
>> -	return component_add(&pdev->dev, &a3xx_ops);
>> +	if (msm_gpu_use_separate_drm_dev(pdev)) {
>> +		ret = msm_gpu_probe(pdev);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = component_add(&pdev->dev, &a3xx_ops);
>> +	if (ret && msm_gpu_use_separate_drm_dev(pdev))
>> +		msm_gpu_remove(pdev);
> 
> What about making it more clear:
> 
> 	if (!msm_gpu_use_separate_drm_dev(pdev))
> 		return component_add(&pdev->dev, &a3xx_ops);
> 
> 	ret = msm_gpu_probe(pdev);
> 	if (ret)
> 		return ret;
> 
> 	ret = component_add(&pdev->dev, &a3xx_ops);
> 
> 	if (ret)
> 		msm_gpu_remove(pdev);
> 
> 	return ret;

Ack. this is more readable.

> 
> However with this patch in place, maybe it's easier to use the main
> msm_drv_probe()? It would need some fixes to handle !kms case as the
> GPU-only, but it looks very similar to your new functions.
>

Agree. Will do this in a separate patch in the next rev.

> 
>> +
>> +	return ret;
>>  }
>>  
>>  static void adreno_remove(struct platform_device *pdev)
>>  {
>> +	component_del(&pdev->dev, &a3xx_ops);
>>  	if (msm_gpu_use_separate_drm_dev(pdev))
>> -		msm_gpu_remove(pdev, &a3xx_ops);
>> -	else
>> -		component_del(&pdev->dev, &a3xx_ops);
>> +		msm_gpu_remove(pdev);
>>  }
>>  
>>  static void adreno_shutdown(struct platform_device *pdev)
>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
>> index b61deafd02c3..af5aa7ff6179 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.c
>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>> @@ -1097,10 +1097,25 @@ int msm_drv_probe(struct device *master_dev,
>>  	return 0;
>>  }
>>  
>> -int msm_gpu_probe(struct platform_device *pdev,
>> -		  const struct component_ops *ops)
>> +static int msm_gpu_drm_bind(struct device *dev)
>> +{
>> +	return msm_drm_init(dev, &msm_gpu_driver, NULL);
> 
> With this patch in place, we can remove the ops argument from
> msm_drm_init() and msm_drm_uninit().

I didn't notice this. Will drop the unused arg. Thanks.

-Akhil.

> 
>> +}
>> +
>> +static void msm_gpu_drm_unbind(struct device *dev)
>> +{
>> +	msm_drm_uninit(dev, NULL);
>> +}
>> +
>> +static const struct component_master_ops msm_gpu_drm_ops = {
>> +	.bind = msm_gpu_drm_bind,
>> +	.unbind = msm_gpu_drm_unbind,
>> +};
>> +
>> +int msm_gpu_probe(struct platform_device *pdev)
>>  {
>>  	struct msm_drm_private *priv;
>> +	struct component_match *match = NULL;
>>  	int ret;
>>  
>>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> @@ -1116,13 +1131,21 @@ int msm_gpu_probe(struct platform_device *pdev,
>>  	if (ret)
>>  		return ret;
>>  
>> -	return msm_drm_init(&pdev->dev, &msm_gpu_driver, ops);
>> +	/*
>> +	 * The GPU pdev acts as both the component master and the sole
>> +	 * component (added by adreno_probe()). Future patches add the
>> +	 * GMU node as a second component on this same master.
>> +	 */
>> +	drm_of_component_match_add(&pdev->dev, &match,
>> +				   component_compare_of, pdev->dev.of_node);
>> +
>> +	return component_master_add_with_match(&pdev->dev, &msm_gpu_drm_ops,
>> +					       match);
>>  }
>>  
>> -void msm_gpu_remove(struct platform_device *pdev,
>> -		    const struct component_ops *ops)
>> +void msm_gpu_remove(struct platform_device *pdev)
>>  {
>> -	msm_drm_uninit(&pdev->dev, ops);
>> +	component_master_del(&pdev->dev, &msm_gpu_drm_ops);
>>  }
>>  
>>  static int __init msm_drm_register(void)
>> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
>> index 6fcb696ceb7c..6264ff27496f 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.h
>> +++ b/drivers/gpu/drm/msm/msm_drv.h
>> @@ -544,10 +544,8 @@ extern const struct component_master_ops msm_drm_ops;
>>  int msm_kms_pm_prepare(struct device *dev);
>>  void msm_kms_pm_complete(struct device *dev);
>>  
>> -int msm_gpu_probe(struct platform_device *pdev,
>> -		  const struct component_ops *ops);
>> -void msm_gpu_remove(struct platform_device *pdev,
>> -		    const struct component_ops *ops);
>> +int msm_gpu_probe(struct platform_device *pdev);
>> +void msm_gpu_remove(struct platform_device *pdev);
>>  int msm_drv_probe(struct device *dev,
>>  	int (*kms_init)(struct drm_device *dev),
>>  	struct msm_kms *kms);
>>
>> -- 
>> 2.51.0
>>
> 

Claude review: drm/msm: Always use component model for standalone GPU

[Claude Code Review Bot]

Patch Review

**Verdict: The design change is sound, with a minor concern.**

This patch converts the standalone GPU path from a shortcut that bypasses the component framework into a proper component master + component setup. The GPU pdev becomes both the component master and a component of itself, preparing for the GMU to be added as a second component in patch 5.

The `msm_gpu_probe` changes look correct:

```c
+	ret = add_gpu_components(&pdev->dev, &match);
+	if (ret)
+		return ret;
+
+	return component_master_add_with_match(&pdev->dev, &msm_gpu_drm_ops,
+					       match);
```

Wait -- at this point in the series (patch 4, before patch 5), `add_gpu_components` only adds the GPU node itself via `drm_of_component_match_add`. But looking at the patched `add_gpu_components` from patch 4's diff... actually patch 4 replaces the inline `drm_of_component_match_add` with a call to `add_gpu_components`. But `add_gpu_components` at this stage (before patch 5) still only adds the GPU node. However, patch 5 modifies `add_gpu_components` to also add the GMU node. So this is fine as a building block.

The error path in `adreno_probe` is correct:

```c
+	ret = component_add(&pdev->dev, &a3xx_ops);
+	if (ret && msm_gpu_use_separate_drm_dev(pdev))
+		msm_gpu_remove(pdev);
```

The comment about "Future patches add the GMU node as a second component" is appropriate but is removed in patch 5, which is good.

---

---
Generated by Claude Code Patch Reviewer

[PATCH RFT 5/5] drm/msm: Attach a driver to the GMU

[Akhil P Oommen]

With the introduction of sync_state in the genpd framework, any consumer
device of GCC and GPUCC which is not bound to a driver will result in
bootup warnings like below:

[   24.362666] gcc-kaanapali 100000.clock-controller: sync_state() pending due to 3d37000.gmu
[   24.371210] gxclkctl-kaanapali 3d64000.clock-controller: sync_state() pending due to 3d37000.gmu
[   24.380268] gpucc-kaanapali 3d90000.clock-controller: sync_state() pending due to 3d37000.gmu

To silence these warnings and also to have a proper state in driver core,
attach a driver to the GMU and set it up as a component device for the drm
master device.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c      | 70 ++++++++++++++++++++++++++++--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c      | 30 +++++--------
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h      |  3 --
 drivers/gpu/drm/msm/adreno/adreno_device.c |  2 +
 drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  3 +-
 drivers/gpu/drm/msm/msm_drv.c              | 17 ++++----
 6 files changed, 92 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index ec13b27feee7..71bb621b01f3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -2063,7 +2063,7 @@ void a6xx_gmu_sysprof_setup(struct msm_gpu *gpu)
 	pm_runtime_put(&gpu->pdev->dev);
 }
 
-void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
+static void a6xx_gmu_destroy(struct a6xx_gpu *a6xx_gpu)
 {
 	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
 	struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
@@ -2143,7 +2143,7 @@ static void __iomem *a6xx_gmu_get_mmio(struct platform_device *pdev, resource_si
 	return ret;
 }
 
-int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
+static int a6xx_gmu_wrapper_pdev_bind(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 {
 	struct platform_device *pdev = of_find_device_by_node(node);
 	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
@@ -2213,6 +2213,8 @@ int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 		goto err_mmio;
 	}
 
+	mutex_init(&gmu->lock);
+
 	gmu->initialized = true;
 
 	return 0;
@@ -2230,7 +2232,7 @@ int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 	return ret;
 }
 
-int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
+static int a6xx_gmu_pdev_bind(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 {
 	struct platform_device *pdev = of_find_device_by_node(node);
 	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
@@ -2415,6 +2417,7 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 	/* Initialize RPMh */
 	a6xx_gmu_rpmh_init(gmu);
 
+	mutex_init(&gmu->lock);
 	gmu->initialized = true;
 
 	return 0;
@@ -2444,3 +2447,64 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 
 	return ret;
 }
+
+static int a6xx_gmu_bind(struct device *dev, struct device *master, void *data)
+{
+	int (*bind)(struct a6xx_gpu *gpu, struct device_node *node) = of_device_get_match_data(dev);
+	struct a6xx_gpu *a6xx_gpu = dev_get_drvdata(dev);
+
+	if (WARN_ON(!a6xx_gpu))
+		return -ENODEV;
+
+	return bind(a6xx_gpu, dev->of_node);
+}
+
+static void a6xx_gmu_unbind(struct device *dev, struct device *master, void *data)
+{
+	struct a6xx_gpu *a6xx_gpu = dev_get_drvdata(dev);
+
+	a6xx_gmu_destroy(a6xx_gpu);
+	dev_set_drvdata(dev, NULL);
+}
+
+static const struct component_ops a6xx_gmu_bind_ops = {
+	.bind   = a6xx_gmu_bind,
+	.unbind = a6xx_gmu_unbind,
+};
+
+static int a6xx_gmu_probe(struct platform_device *pdev)
+{
+	return component_add(&pdev->dev, &a6xx_gmu_bind_ops);
+}
+
+static void a6xx_gmu_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &a6xx_gmu_bind_ops);
+}
+
+static const struct of_device_id a6xx_gmu_dt_match[] = {
+	{ .compatible = "qcom,adreno-gmu", .data = a6xx_gmu_pdev_bind },
+	{ .compatible = "qcom,adreno-rgmu", .data = a6xx_gmu_wrapper_pdev_bind },
+	{ .compatible = "qcom,adreno-gmu-wrapper", .data = a6xx_gmu_wrapper_pdev_bind },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, a6xx_gmu_dt_match);
+
+static struct platform_driver adreno_gmu_driver = {
+	.probe = a6xx_gmu_probe,
+	.remove = a6xx_gmu_remove,
+	.driver = {
+		.name = "adreno_gmu",
+		.of_match_table = a6xx_gmu_dt_match,
+	},
+};
+
+void __init adreno_gmu_register(void)
+{
+	platform_driver_register(&adreno_gmu_driver);
+}
+
+void __exit adreno_gmu_unregister(void)
+{
+	platform_driver_unregister(&adreno_gmu_driver);
+}
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index d5aba072f44c..b4ae38e1a3d8 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -10,6 +10,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/devfreq.h>
+#include <linux/of_platform.h>
 #include <linux/pm_domain.h>
 #include <linux/soc/qcom/llcc-qcom.h>
 
@@ -2409,8 +2410,6 @@ static void a6xx_destroy(struct msm_gpu *gpu)
 
 	a6xx_llc_slices_destroy(a6xx_gpu);
 
-	a6xx_gmu_remove(a6xx_gpu);
-
 	adreno_gpu_cleanup(adreno_gpu);
 
 	kfree(a6xx_gpu);
@@ -2622,6 +2621,7 @@ static struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
 	struct adreno_platform_config *config = pdev->dev.platform_data;
 	const struct adreno_info *info = config->info;
 	struct device_node *node;
+	struct platform_device *gmu_pdev;
 	struct a6xx_gpu *a6xx_gpu;
 	struct adreno_gpu *adreno_gpu;
 	struct msm_gpu *gpu;
@@ -2637,17 +2637,14 @@ static struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
 	adreno_gpu = &a6xx_gpu->base;
 	gpu = &adreno_gpu->base;
 
-	mutex_init(&a6xx_gpu->gmu.lock);
 	spin_lock_init(&a6xx_gpu->aperture_lock);
 
 	adreno_gpu->registers = NULL;
 
-	/* Check if there is a GMU phandle and set it up */
 	node = of_parse_phandle(pdev->dev.of_node, "qcom,gmu", 0);
-	/* FIXME: How do we gracefully handle this? */
-	BUG_ON(!node);
-
-	adreno_gpu->gmu_is_wrapper = of_device_is_compatible(node, "qcom,adreno-gmu-wrapper");
+	WARN_ON(!node);
+	adreno_gpu->gmu_is_wrapper = of_device_is_compatible(node,
+							     "qcom,adreno-gmu-wrapper");
 
 	adreno_gpu->base.hw_apriv =
 		!!(info->quirks & ADRENO_QUIRK_HAS_HW_APRIV);
@@ -2686,16 +2683,6 @@ static struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
 	if (adreno_is_a618(adreno_gpu) || adreno_is_7c3(adreno_gpu))
 		priv->gpu_clamp_to_idle = true;
 
-	if (adreno_has_gmu_wrapper(adreno_gpu) || adreno_has_rgmu(adreno_gpu))
-		ret = a6xx_gmu_wrapper_init(a6xx_gpu, node);
-	else
-		ret = a6xx_gmu_init(a6xx_gpu, node);
-	of_node_put(node);
-	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,
@@ -2710,6 +2697,13 @@ static struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
 	/* Set up the preemption specific bits and pieces for each ringbuffer */
 	a6xx_preempt_init(gpu);
 
+	gmu_pdev = of_find_device_by_node(node);
+	of_node_put(node);
+	if (gmu_pdev) {
+		platform_set_drvdata(gmu_pdev, a6xx_gpu);
+		put_device(&gmu_pdev->dev);
+	}
+
 	return gpu;
 }
 
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
index eb431e5e00b1..fe8d68df9944 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
@@ -268,9 +268,6 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu);
 int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state);
 void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state);
 
-int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node);
-int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node);
-void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu);
 void a6xx_gmu_sysprof_setup(struct msm_gpu *gpu);
 
 void a6xx_preempt_init(struct msm_gpu *gpu);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 67686424f3a1..d70f98e283d5 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -423,6 +423,7 @@ void __init adreno_register(void)
 		return;
 
 	platform_driver_register(&adreno_driver);
+	adreno_gmu_register();
 }
 
 void __exit adreno_unregister(void)
@@ -430,5 +431,6 @@ void __exit adreno_unregister(void)
 	if (skip_gpu)
 		return;
 
+	adreno_gmu_unregister();
 	platform_driver_unregister(&adreno_driver);
 }
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index ec643b84646b..3a05cd98d215 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -642,7 +642,8 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
 int adreno_gpu_state_put(struct msm_gpu_state *state);
 void adreno_show_object(struct drm_printer *p, void **ptr, int len,
 		bool *encoded);
-
+void adreno_gmu_register(void);
+void adreno_gmu_unregister(void);
 /*
  * Common helper function to initialize the default address space for arm-smmu
  * attached targets
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index af5aa7ff6179..426255fd0801 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1021,7 +1021,7 @@ static const struct of_device_id msm_gpu_match[] = {
 static int add_gpu_components(struct device *dev,
 			      struct component_match **matchptr)
 {
-	struct device_node *np;
+	struct device_node *np, *gmu_np;
 
 	np = of_find_matching_node(NULL, msm_gpu_match);
 	if (!np)
@@ -1030,6 +1030,11 @@ static int add_gpu_components(struct device *dev,
 	if (of_device_is_available(np) && adreno_has_gpu(np))
 		drm_of_component_match_add(dev, matchptr, component_compare_of, np);
 
+	gmu_np = of_parse_phandle(np, "qcom,gmu", 0);
+	if (of_device_is_available(gmu_np))
+		drm_of_component_match_add(dev, matchptr, component_compare_of, gmu_np);
+
+	of_node_put(gmu_np);
 	of_node_put(np);
 
 	return 0;
@@ -1131,13 +1136,9 @@ int msm_gpu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	/*
-	 * The GPU pdev acts as both the component master and the sole
-	 * component (added by adreno_probe()). Future patches add the
-	 * GMU node as a second component on this same master.
-	 */
-	drm_of_component_match_add(&pdev->dev, &match,
-				   component_compare_of, pdev->dev.of_node);
+	ret = add_gpu_components(&pdev->dev, &match);
+	if (ret)
+		return ret;
 
 	return component_master_add_with_match(&pdev->dev, &msm_gpu_drm_ops,
 					       match);

-- 
2.51.0

Re: [PATCH RFT 5/5] drm/msm: Attach a driver to the GMU

[Dmitry Baryshkov]

On Wed, May 13, 2026 at 02:23:00AM +0530, Akhil P Oommen wrote:
> With the introduction of sync_state in the genpd framework, any consumer
> device of GCC and GPUCC which is not bound to a driver will result in
> bootup warnings like below:
> 
> [   24.362666] gcc-kaanapali 100000.clock-controller: sync_state() pending due to 3d37000.gmu
> [   24.371210] gxclkctl-kaanapali 3d64000.clock-controller: sync_state() pending due to 3d37000.gmu
> [   24.380268] gpucc-kaanapali 3d90000.clock-controller: sync_state() pending due to 3d37000.gmu
> 
> To silence these warnings and also to have a proper state in driver core,
> attach a driver to the GMU and set it up as a component device for the drm
> master device.
> 
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c      | 70 ++++++++++++++++++++++++++++--
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c      | 30 +++++--------
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.h      |  3 --
>  drivers/gpu/drm/msm/adreno/adreno_device.c |  2 +
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h    |  3 +-
>  drivers/gpu/drm/msm/msm_drv.c              | 17 ++++----
>  6 files changed, 92 insertions(+), 33 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Claude review: drm/msm: Attach a driver to the GMU

[Claude Code Review Bot]

Patch Review

**Verdict: The approach works but has a fragile drvdata handoff that should be reconsidered.**

This is the main patch. It registers a platform driver for the GMU, making it a component device on the same master as the GPU.

**Issue 1 -- Cross-component drvdata coupling creates a dangling pointer risk:**

In `a6xx_gpu_init`, the GPU component's bind sets the GMU pdev's drvdata:

```c
	gmu_pdev = of_find_device_by_node(node);
	of_node_put(node);
	if (gmu_pdev) {
		platform_set_drvdata(gmu_pdev, a6xx_gpu);
		put_device(&gmu_pdev->dev);
	}
```

Then in `a6xx_gmu_bind`, the GMU component reads it back:

```c
	struct a6xx_gpu *a6xx_gpu = dev_get_drvdata(dev);
```

If the GMU component's bind fails, `component_bind_all` will unwind by calling the GPU component's unbind, which calls `a6xx_destroy` -> `kfree(a6xx_gpu)`. But the GMU pdev's drvdata still points to the freed `a6xx_gpu` -- it was never cleared because `a6xx_gmu_unbind` was never called (the GMU bind failed, not succeeded). This leaves a dangling pointer.

In practice it may be benign since nothing accesses the GMU pdev's drvdata after the master bind fails, but it's still a latent use-after-free. Consider clearing the GMU's drvdata in the GPU's unbind/destroy path, or using a different mechanism (e.g., passing the `a6xx_gpu` pointer through the component `data` parameter from the master).

**Issue 2 -- `BUG_ON` to `WARN_ON` without early return:**

```c
	node = of_parse_phandle(pdev->dev.of_node, "qcom,gmu", 0);
-	/* FIXME: How do we gracefully handle this? */
-	BUG_ON(!node);
+	WARN_ON(!node);
```

While removing `BUG_ON` is good, if `node` is NULL the function continues and eventually passes `NULL` to `of_find_device_by_node`, `of_node_put`, etc. These happen to be NULL-safe, and `of_device_is_compatible(NULL, ...)` returns 0, so it won't crash. But the GPU will be initialized without a GMU, which will cause problems later at runtime. This should be an early `return ERR_PTR(-ENODEV)` instead of a bare `WARN_ON`.

**Issue 3 -- GMU init/destroy no longer called from a6xx_gpu_init/a6xx_destroy:**

The original code had:

```c
-	if (adreno_has_gmu_wrapper(adreno_gpu) || adreno_has_rgmu(adreno_gpu))
-		ret = a6xx_gmu_wrapper_init(a6xx_gpu, node);
-	else
-		ret = a6xx_gmu_init(a6xx_gpu, node);
```

And `a6xx_destroy` had:

```c
-	a6xx_gmu_remove(a6xx_gpu);
```

These are now handled by the GMU component's bind/unbind. This means the GMU init/destroy ordering is now determined by the component framework rather than being explicitly controlled. The ordering relies on the GPU being listed before the GMU in the match list, and unbinding happening in reverse order. This is correct behavior for the component framework, but it's implicit rather than explicit -- worth a comment.

**Issue 4 -- `adreno_gmu_register()` / `adreno_gmu_unregister()` lack `__init`/`__exit` annotations in the header:**

```c
+void adreno_gmu_register(void);
+void adreno_gmu_unregister(void);
```

The implementations have `__init`/`__exit`:

```c
+void __init adreno_gmu_register(void)
+void __exit adreno_gmu_unregister(void)
```

The declarations in `adreno_gpu.h` should also carry the `__init`/`__exit` annotations to avoid section mismatch warnings when building with certain configs.

**Issue 5 -- Error return from `platform_driver_register` is ignored:**

```c
+void __init adreno_gmu_register(void)
+{
+	platform_driver_register(&adreno_gmu_driver);
+}
```

The return value is silently discarded. This matches the pattern used by `adreno_register` itself (which also ignores `platform_driver_register`'s return), so it's consistent, but it's worth noting.

**Minor -- `add_gpu_components` now always adds the GMU to the component match:**

In the non-standalone path, `add_gpu_components` is called from `msm_drv_probe` when `!msm_gpu_use_separate_drm_dev()`. This means the main msm_drm component master now waits for the GMU component in addition to GPU + display components. This is the intended behavior but represents a behavioral change for all a6xx+ SoCs with KMS, not just standalone GPU configurations. If the GMU driver fails to probe for any reason, the entire DRM master will fail to bind. Make sure this is tested with KMS-enabled SoCs (the common case), not just standalone GPU configurations.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm: Attach a driver to GMU

[Claude Code Review Bot]

Overall Series Review

Subject: drm/msm: Attach a driver to GMU
Author: Akhil P Oommen <akhilpo@oss.qualcomm.com>
Patches: 13
Reviewed: 2026-05-16T12:47:07.090237

---

This 5-patch series addresses `sync_state()` warnings from genpd when the GMU device node has no driver bound. The approach is to register a proper platform driver for the GMU and integrate it as a component device alongside the GPU in the DRM component model.

The series is logically structured: patches 1-3 are preparatory fixes, patch 4 restructures the standalone GPU path to always use the component framework, and patch 5 introduces the GMU driver. The overall direction is sound, but the key mechanism in patch 5 -- passing the `a6xx_gpu` pointer to the GMU bind callback via `platform_set_drvdata` on another device -- is fragile and introduces a cross-component coupling that could lead to a dangling pointer on error paths. This should be addressed before merging.

The series is marked RFT (Request For Testing) which is appropriate given the scope of changes.

---

---
Generated by Claude Code Patch Reviewer