From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/msm: enable separate_gpu_kms by default
Date: Mon, 23 Feb 2026 05:46:34 +1000 [thread overview]
Message-ID: <review-patch1-20260223-seperate_gpu_kms-v1-1-e8231e7f1685@oss.qualcomm.com> (raw)
In-Reply-To: <20260223-seperate_gpu_kms-v1-1-e8231e7f1685@oss.qualcomm.com>
Patch Review
> -static bool separate_gpu_kms;
> +static bool separate_gpu_kms = true;
> 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)");
The `MODULE_PARM_DESC` text says "0=single DRM device for both GPU and display (default)" but after this patch the default is now 1 (separate). The description should be updated to reflect the new default, something like "1=two DRM devices (default)" and removing "(default)" from the 0 option.
Additionally, there is a pre-existing naming mismatch: `MODULE_PARM_DESC` refers to `separate_gpu_drm` while `module_param` uses `separate_gpu_kms`. This means the description visible via `modinfo` is associated with a parameter name (`separate_gpu_drm`) that doesn't actually exist as a parameter. Since this patch is already touching this line, it would be a good opportunity to fix the `MODULE_PARM_DESC` name to match `separate_gpu_kms`.
The commit message mentions this is needed for SA8775P targets with multiple display subsystems, but doesn't discuss potential regressions on existing single-display targets. Changing a default like this affects all MSM platforms. Is there any risk of userspace breakage on platforms that currently expect a single DRM device? If userspace (e.g., display managers, compositors) on existing deployed systems is configured to find the GPU via the combined DRM device node, changing the default could break those setups. Some discussion of backwards compatibility or testing coverage in the commit message would be helpful.
The patch declares dependencies on two other series. Does this patch function correctly without those dependencies, or would applying it standalone cause probe failures? The commit message should clarify this.
---
Generated by Claude Code Patch Reviewer
prev parent reply other threads:[~2026-02-22 19:46 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-22 19:10 [PATCH] drm/msm: enable separate_gpu_kms by default Mahadevan P
2026-02-22 19:46 ` Claude review: " Claude Code Review Bot
2026-02-22 19:46 ` Claude Code Review Bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=review-patch1-20260223-seperate_gpu_kms-v1-1-e8231e7f1685@oss.qualcomm.com \
--to=claude-review@example.com \
--cc=dri-devel-reviews@example.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox