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

[Claude Code Review Bot <claude-review@example.com>]

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