[PATCH v3] drm/msm: default separate_gpu_kms to auto selection

[PATCH v3] drm/msm: default separate_gpu_kms to auto selection

[Mahadevan P]

On platforms with multiple display subsystems, such as SA8775P, the GPU
binds to the first display subsystem that probes. This implicit binding
prevents subsequent display subsystems from probing successfully,
breaking multi-display support.

Use the tristate separate_gpu_kms module parameter with the default
value set to auto (-1). In auto mode, the driver selects the binding
behavior based on the number of GPUs and display subsystems. This allows
display subsystems to probe independently when required, while
preserving the existing single-card behavior on simpler systems.

The separate_gpu_kms module parameter has the following semantics:

  -1 (auto, default):
     Select the binding mode based on hardware topology. If exactly one
     GPU and one display subsystem are present, bind them together to
     form a single DRM device. Otherwise, expose the GPU and display
     subsystems as separate DRM devices.

   0:
     Always bind the GPU and display together to form a single DRM
     device.

   1:
     Always expose the GPU and display subsystems as separate DRM
     devices.

Additionally, ensure that display subsystems are always exposed as
separate DRM devices when no recognized Adreno GPU is present,
regardless of the separate_gpu_kms setting.

This ensures correct probing on multi-display platforms without
affecting single-display, single-GPU systems.

Signed-off-by: Mahadevan P <mahadevan.p@oss.qualcomm.com>
---
Depends on:
  https://lore.kernel.org/lkml/20260124-adreno-module-table-v1-1-9c2dbb2638b4@oss.qualcomm.com/

  When separate_gpu_kms is enabled, the GPU and display drivers are
  probed independently. In this configuration, the Adreno GPU driver
  may no longer be loaded implicitly via the display subsystem.

  The referenced patch adds a MODULE_DEVICE_TABLE() entry for the
  Adreno GPU device, ensuring proper module autoloading based on
  device tree matching. This is required to guarantee that the GPU
  driver is loaded correctly when GPU and display probing are
  decoupled.

Changes in v3:
- Ensure display subsystems are always exposed as separate DRM devices when
  no recognized Adreno GPU is present (!adreno_has_gpu()), regardless of
  separate_gpu_kms setting. (Dmitry)
- Extend auto-selection logic to account for legacy display controllers (MDP4 and MDP5). (Dmitry)
- Rename msm_mdss_count_masters() to msm_mdss_count() to better reflect functionality. (Dmitry)
- Common helper to determine presence of a valid Adreno GPU node, avoiding repeated
  adreno_has_gpu() and availability checks.
- Update commit message to reflect the above behavior.
- Link to v2: https://lore.kernel.org/r/20260317-separate_gpu_kms-v2-1-b027ca97b9fe@oss.qualcomm.com

Changes in v2:
- Drop dependency on Lemans dual-DPU device tree changes as this patch
  works independently (Dmitry)
- Switch separate_gpu_kms to tristate and default to auto mode (Rob)
- Rename msm_gpu_no_components() to msm_separate_gpu_kms_components() for clarity
- Link to v1: https://lore.kernel.org/r/20260223-seperate_gpu_kms-v1-1-e8231e7f1685@oss.qualcomm.com
---
 drivers/gpu/drm/msm/adreno/adreno_device.c |  2 +-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c   | 14 +++++++
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c   | 14 +++++++
 drivers/gpu/drm/msm/msm_drv.c              | 64 ++++++++++++++++++++++++++----
 drivers/gpu/drm/msm/msm_drv.h              |  8 +++-
 drivers/gpu/drm/msm/msm_mdss.c             | 15 +++++++
 6 files changed, 107 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 4edfe80c5be7..e40648c05797 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -272,7 +272,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())
+	    msm_separate_gpu_kms_components())
 		return msm_gpu_probe(pdev, &a3xx_ops);
 
 	return component_add(&pdev->dev, &a3xx_ops);
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 809ca191e9de..409000b739b6 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -15,6 +15,20 @@
 #include "msm_mmu.h"
 #include "mdp4_kms.h"
 
+static const struct of_device_id mdp4_dt_match[];
+int msm_count_mdp4(void)
+{
+	struct device_node *np;
+	int count = 0;
+
+	for_each_matching_node(np, mdp4_dt_match) {
+		if (of_device_is_available(np))
+			count++;
+	}
+
+	return count;
+}
+
 static int mdp4_hw_init(struct msm_kms *kms)
 {
 	struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 1e3dc9bf9494..086895c9f103 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -19,6 +19,20 @@
 #include "msm_mmu.h"
 #include "mdp5_kms.h"
 
+static const struct of_device_id mdp5_dt_match[];
+int msm_count_mdp5(void)
+{
+	struct device_node *np;
+	int count = 0;
+
+	for_each_matching_node(np, mdp5_dt_match) {
+		if (of_device_is_available(np))
+			count++;
+	}
+
+	return count;
+}
+
 static int mdp5_hw_init(struct msm_kms *kms)
 {
 	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index e5ab1e28851d..e2101c6632ac 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -54,16 +54,64 @@ static bool modeset = true;
 MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (1=on (default), 0=disable)");
 module_param(modeset, bool, 0600);
 
-static bool separate_gpu_kms;
-MODULE_PARM_DESC(separate_gpu_drm, "Use separate DRM device for the GPU (0=single DRM device for both GPU and display (default), 1=two DRM devices)");
-module_param(separate_gpu_kms, bool, 0400);
+/*
+ * separate_gpu_kms (tristate):
+ *   -1 (default): decide automatically based on hardware topology. Split devices
+ *                 if there is more than one GPU or more than one display master.
+ *    0: force single DRM device (bind display + GPU)
+ *    1: force separate DRM devices
+ */
+static int separate_gpu_kms = -1;
+MODULE_PARM_DESC(separate_gpu_kms,
+		 "Use separate DRM device for the GPU (-1=auto (default), 0=single DRM device, 1=separate DRM devices)");
+module_param(separate_gpu_kms, int, 0400);
 
 DECLARE_FAULT_ATTR(fail_gem_alloc);
 DECLARE_FAULT_ATTR(fail_gem_iova);
 
-bool msm_gpu_no_components(void)
+static const struct of_device_id msm_gpu_match[];
+static inline bool msm_gpu_node_present(struct device_node *np)
+{
+	return np && of_device_is_available(np) && adreno_has_gpu(np);
+}
+
+static int msm_count_gpus(void)
+{
+	struct device_node *np;
+	int count = 0;
+
+	for_each_matching_node(np, msm_gpu_match) {
+		if (msm_gpu_node_present(np))
+			count++;
+	}
+
+	return count;
+}
+
+static bool msm_separate_gpu_kms_auto(void)
+{
+	int gpus = msm_count_gpus();
+	int display_subsystems = msm_count_mdss() + msm_count_mdp4() + msm_count_mdp5();
+
+	if (gpus <= 0 || display_subsystems <= 0)
+		return false;
+
+	/* If exactly one GPU and one display subsystem single card */
+	return (gpus > 1) || (display_subsystems > 1);
+}
+
+bool msm_separate_gpu_kms_components(void)
 {
-	return separate_gpu_kms;
+	struct device_node *np;
+
+	np = of_find_matching_node(NULL, msm_gpu_match);
+	if (!msm_gpu_node_present(np))
+		return true;
+	if (separate_gpu_kms == 1)
+		return true;
+	if (separate_gpu_kms == 0)
+		return false;
+	return msm_separate_gpu_kms_auto();
 }
 
 static int msm_drm_uninit(struct device *dev, const struct component_ops *gpu_ops)
@@ -1019,7 +1067,7 @@ static int add_gpu_components(struct device *dev,
 	if (!np)
 		return 0;
 
-	if (of_device_is_available(np) && adreno_has_gpu(np))
+	if (msm_gpu_node_present(np))
 		drm_of_component_match_add(dev, matchptr, component_compare_of, np);
 
 	of_node_put(np);
@@ -1030,7 +1078,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_separate_gpu_kms_components() ?
 				    &msm_kms_driver :
 				    &msm_driver,
 			    NULL);
@@ -1069,7 +1117,7 @@ int msm_drv_probe(struct device *master_dev,
 			return ret;
 	}
 
-	if (!msm_gpu_no_components()) {
+	if (!msm_separate_gpu_kms_components()) {
 		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..a977fe7b36b8 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -555,6 +555,12 @@ 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_separate_gpu_kms_components(void);
+
+int msm_count_mdss(void);
+
+int msm_count_mdp4(void);
+
+int msm_count_mdp5(void);
 
 #endif /* __MSM_DRV_H__ */
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 9047e8d9ee89..4c788f2647b0 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -73,6 +73,21 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
 	return 0;
 }
 
+static const struct of_device_id mdss_dt_match[];
+
+int msm_count_mdss(void)
+{
+	struct device_node *np;
+	int count = 0;
+
+	for_each_matching_node(np, mdss_dt_match) {
+		if (of_device_is_available(np))
+			count++;
+	}
+
+	return count;
+}
+
 static void msm_mdss_irq(struct irq_desc *desc)
 {
 	struct msm_mdss *msm_mdss = irq_desc_get_handler_data(desc);

---
base-commit: b84a0ebe421ca56995ff78b66307667b62b3a900
change-id: 20260316-separate_gpu_kms-04d2cf4d91e2

Best regards,
-- 
Mahadevan P <mahadevan.p@oss.qualcomm.com>

Claude review: drm/msm: default separate_gpu_kms to auto selection

[Claude Code Review Bot]

Overall Series Review

Subject: drm/msm: default separate_gpu_kms to auto selection
Author: Mahadevan P <mahadevan.p@oss.qualcomm.com>
Patches: 1
Reviewed: 2026-03-31T17:21:55.073456

---

This is a single patch (v3) that changes the `separate_gpu_kms` module parameter from a boolean to a tristate (`-1`/`0`/`1`), adding an auto-detection mode that counts GPUs and display subsystems to decide whether to bind them together or expose them as separate DRM devices. The motivation is clear: platforms like SA8775P with multiple display subsystems need them to probe independently.

The design is reasonable and the auto-detection logic is sound. However, there is a **device node reference leak** that must be fixed before this can be merged, and a few minor points worth addressing.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm: default separate_gpu_kms to auto selection

[Claude Code Review Bot]

Patch Review

**Bug: `of_node_put()` missing in `msm_separate_gpu_kms_components()`**

This is the most significant issue. `of_find_matching_node()` returns a node with an incremented refcount, but the function never calls `of_node_put()`:

```c
bool msm_separate_gpu_kms_components(void)
{
	struct device_node *np;

	np = of_find_matching_node(NULL, msm_gpu_match);
	if (!msm_gpu_node_present(np))
		return true;
	if (separate_gpu_kms == 1)
		return true;
	if (separate_gpu_kms == 0)
		return false;
	return msm_separate_gpu_kms_auto();
}
```

Every return path after the `of_find_matching_node()` call leaks the node reference. This function is called from multiple call sites (`adreno_probe`, `msm_drm_bind`, `msm_drv_probe`), so the leak compounds. An `of_node_put(np)` is needed before every `return`. A common pattern would be:

```c
bool msm_separate_gpu_kms_components(void)
{
	struct device_node *np;
	bool ret;

	np = of_find_matching_node(NULL, msm_gpu_match);
	if (!msm_gpu_node_present(np)) {
		of_node_put(np);
		return true;
	}
	of_node_put(np);

	if (separate_gpu_kms == 1)
		return true;
	if (separate_gpu_kms == 0)
		return false;
	return msm_separate_gpu_kms_auto();
}
```

Note that `of_node_put(NULL)` is safe, so the `!msm_gpu_node_present(np)` path can call it unconditionally. After the present-check, `np` is no longer used, so the put can happen right after.

**Multiple calls to `msm_separate_gpu_kms_components()` re-scan device tree each time**

This function is called from at least three sites during probe. Each call does `of_find_matching_node` + `for_each_matching_node` (via `msm_count_gpus`) + three more `for_each_matching_node` calls (via `msm_count_mdss/mdp4/mdp5`). Consider caching the result after the first evaluation, since the hardware topology doesn't change at runtime. This is a minor performance concern, not a correctness issue.

**Forward declaration of `mdp4_dt_match[]` and `mdp5_dt_match[]`**

```c
static const struct of_device_id mdp4_dt_match[];
int msm_count_mdp4(void)
```

These forward-declare `static` arrays from later in the same file. This works but some compilers may warn about incomplete array types. It's a pre-existing pattern in the MSM driver (already done for `mdss_dt_match` and `msm_gpu_match`), so it's consistent.

**`msm_count_mdp4()` and `msm_count_mdp5()` are not `static` but lack declarations in msm_drv.h prior to patch**

The patch correctly adds declarations in `msm_drv.h`:
```c
int msm_count_mdp4(void);
int msm_count_mdp5(void);
```
This is fine.

**Minor: `msm_separate_gpu_kms_auto()` returns `false` when `gpus <= 0`**

```c
if (gpus <= 0 || display_subsystems <= 0)
    return false;
```

When no GPU is found (`gpus == 0`), this returns `false` (don't separate). But `msm_separate_gpu_kms_components()` already handles the no-GPU case by returning `true` (separate) before reaching auto. So this path in `msm_separate_gpu_kms_auto()` is effectively dead for the `gpus <= 0` case. Not a bug, but the comment `/* If exactly one GPU and one display subsystem single card */` could note this nuance.

**Behavioral consideration: `separate_gpu_kms=0` overrides no-GPU detection**

When a user forces `separate_gpu_kms=0` but no recognized Adreno GPU is present, `msm_separate_gpu_kms_components()` returns `true` (separate) regardless — because the `!msm_gpu_node_present(np)` check comes before the `separate_gpu_kms == 0` check. This is documented behavior ("ensure display subsystems are always exposed as separate DRM devices when no recognized Adreno GPU is present, regardless of the separate_gpu_kms setting"), but it means the module parameter can't actually force combined mode on systems without a recognized GPU. This is probably the right call, but worth a comment in the code to explain why the parameter is ignored in this case.

**Summary**: The of_node_put leak is a real bug that needs fixing. The overall design and auto-detection logic are clean and well-structured. With the refcount leak fixed, this looks ready to merge.

---
Generated by Claude Code Patch Reviewer