public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] fixes to DRM doc & parameter's print
@ 2026-05-22 13:55 Michał Grzelak
  2026-05-22 13:55 ` [PATCH v2 1/2] drm/print: describe 6th & 9th bit of drm.debug Michał Grzelak
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Michał Grzelak @ 2026-05-22 13:55 UTC (permalink / raw)
  To: dri-devel, intel-gfx, intel-xe
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Chaitanya Kumar Borah, Michał Grzelak

Next version of [1]. Changed commit message to mention kernel-doc of
actually changed macro.

[1] https://lore.kernel.org/intel-gfx/20260521155231.1821935-1-michal.grzelak@intel.com/

BR,
Michał

Michał Grzelak (2):
  drm/print: describe 6th & 9th bit of drm.debug
  drm/managed: fix drmm_add_action() kernel-doc

 drivers/gpu/drm/drm_print.c | 4 +++-
 include/drm/drm_managed.h   | 2 +-
 include/drm/drm_print.h     | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.45.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] drm/print: describe 6th & 9th bit of drm.debug
  2026-05-22 13:55 [PATCH v2 0/2] fixes to DRM doc & parameter's print Michał Grzelak
@ 2026-05-22 13:55 ` Michał Grzelak
  2026-05-22 19:55   ` Michał Grzelak
  2026-05-25  8:29   ` Claude review: " Claude Code Review Bot
  2026-05-22 13:55 ` [PATCH v2 2/2] drm/managed: fix drmm_add_action() kernel-doc Michał Grzelak
  2026-05-25  8:29 ` Claude review: fixes to DRM doc & parameter's print Claude Code Review Bot
  2 siblings, 2 replies; 8+ messages in thread
From: Michał Grzelak @ 2026-05-22 13:55 UTC (permalink / raw)
  To: dri-devel, intel-gfx, intel-xe
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Chaitanya Kumar Borah, Michał Grzelak

Setting 6th or 9th bit of drm.debug change debug logging. Meanwhile
`modinfo drm` does not inform about it at all.

Add info to MODULE_PARAM_DESC(debug, ...) about setting 6th and 9th bit
basing on DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, ...). Match
description of corresponding bits with enum drm_debug_category. Include
9th bit in the example with enabling all possible logging provided at
comment at include/drm/drm_print.h.

Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>
---
 drivers/gpu/drm/drm_print.c | 4 +++-
 include/drm/drm_print.h     | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index ded9461df5f2..86cef1a37678 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -50,8 +50,10 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
 "\t\tBit 3 (0x08)  will enable PRIME messages (prime code)\n"
 "\t\tBit 4 (0x10)  will enable ATOMIC messages (atomic code)\n"
 "\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n"
+"\t\tBit 6 (0x40)  will enable STATE messages (atomic state code)\n"
 "\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n"
-"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
+"\t\tBit 8 (0x100) will enable DP messages (displayport code)\n"
+"\t\tBit 9 (0x200) will enable DRMRES messages (managed resources code)");
 
 #if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
 module_param_named(debug, __drm_debug, ulong, 0600);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index ab017b05e175..2adc5ac688e1 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -87,7 +87,7 @@ extern unsigned long __drm_debug;
  *  - drm.debug=0x2 will enable DRIVER messages
  *  - drm.debug=0x3 will enable CORE and DRIVER messages
  *  - ...
- *  - drm.debug=0x1ff will enable all messages
+ *  - drm.debug=0x3ff will enable all messages
  *
  * An interesting feature is that it's possible to enable verbose logging at
  * run-time by echoing the debug value in its sysfs node::
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] drm/managed: fix drmm_add_action() kernel-doc
  2026-05-22 13:55 [PATCH v2 0/2] fixes to DRM doc & parameter's print Michał Grzelak
  2026-05-22 13:55 ` [PATCH v2 1/2] drm/print: describe 6th & 9th bit of drm.debug Michał Grzelak
@ 2026-05-22 13:55 ` Michał Grzelak
  2026-05-25  8:29   ` Claude review: " Claude Code Review Bot
  2026-05-25  8:29 ` Claude review: fixes to DRM doc & parameter's print Claude Code Review Bot
  2 siblings, 1 reply; 8+ messages in thread
From: Michał Grzelak @ 2026-05-22 13:55 UTC (permalink / raw)
  To: dri-devel, intel-gfx, intel-xe
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Chaitanya Kumar Borah, Michał Grzelak

Kernel-doc of drmm_add_action() references @releases which is not
on argument list. Swap '@' between 'releases' and 'action' words to fix
the documentation.

Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>
---
 include/drm/drm_managed.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
index 72bfac002c06..72d0d68be226 100644
--- a/include/drm/drm_managed.h
+++ b/include/drm/drm_managed.h
@@ -18,7 +18,7 @@ typedef void (*drmres_release_t)(struct drm_device *dev, void *res);
  * @action: function which should be called when @dev is released
  * @data: opaque pointer, passed to @action
  *
- * This function adds the @release action with optional parameter @data to the
+ * This function adds the release @action with optional parameter @data to the
  * list of cleanup actions for @dev. The cleanup actions will be run in reverse
  * order in the final drm_dev_put() call for @dev.
  */
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drm/print: describe 6th & 9th bit of drm.debug
  2026-05-22 13:55 ` [PATCH v2 1/2] drm/print: describe 6th & 9th bit of drm.debug Michał Grzelak
@ 2026-05-22 19:55   ` Michał Grzelak
  2026-05-25  8:29   ` Claude review: " Claude Code Review Bot
  1 sibling, 0 replies; 8+ messages in thread
From: Michał Grzelak @ 2026-05-22 19:55 UTC (permalink / raw)
  To: Michał Grzelak
  Cc: dri-devel, intel-gfx, intel-xe, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Chaitanya Kumar Borah

[-- Attachment #1: Type: text/plain, Size: 2386 bytes --]

On Fri, 22 May 2026, Michał Grzelak wrote:
> Setting 6th or 9th bit of drm.debug change debug logging. Meanwhile
> `modinfo drm` does not inform about it at all.
>
> Add info to MODULE_PARAM_DESC(debug, ...) about setting 6th and 9th bit
> basing on DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, ...). Match
> description of corresponding bits with enum drm_debug_category. Include
> 9th bit in the example with enabling all possible logging provided at
> comment at include/drm/drm_print.h.
>
> Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>

What should have been added here is R-B: tag provided by Chaitanya,
which currently is very much absent. The omission was not intentional.

BR,
Michał

> ---
> drivers/gpu/drm/drm_print.c | 4 +++-
> include/drm/drm_print.h     | 2 +-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index ded9461df5f2..86cef1a37678 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -50,8 +50,10 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
> "\t\tBit 3 (0x08)  will enable PRIME messages (prime code)\n"
> "\t\tBit 4 (0x10)  will enable ATOMIC messages (atomic code)\n"
> "\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n"
> +"\t\tBit 6 (0x40)  will enable STATE messages (atomic state code)\n"
> "\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n"
> -"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
> +"\t\tBit 8 (0x100) will enable DP messages (displayport code)\n"
> +"\t\tBit 9 (0x200) will enable DRMRES messages (managed resources code)");
>
> #if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
> module_param_named(debug, __drm_debug, ulong, 0600);
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index ab017b05e175..2adc5ac688e1 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -87,7 +87,7 @@ extern unsigned long __drm_debug;
>  *  - drm.debug=0x2 will enable DRIVER messages
>  *  - drm.debug=0x3 will enable CORE and DRIVER messages
>  *  - ...
> - *  - drm.debug=0x1ff will enable all messages
> + *  - drm.debug=0x3ff will enable all messages
>  *
>  * An interesting feature is that it's possible to enable verbose logging at
>  * run-time by echoing the debug value in its sysfs node::
> -- 
> 2.45.2
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: fixes to DRM doc & parameter's print
  2026-05-22 13:55 [PATCH v2 0/2] fixes to DRM doc & parameter's print Michał Grzelak
  2026-05-22 13:55 ` [PATCH v2 1/2] drm/print: describe 6th & 9th bit of drm.debug Michał Grzelak
  2026-05-22 13:55 ` [PATCH v2 2/2] drm/managed: fix drmm_add_action() kernel-doc Michał Grzelak
@ 2026-05-25  8:29 ` Claude Code Review Bot
  2 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  8:29 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: fixes to DRM doc & parameter's print
Author: =?UTF-8?q?Micha=C5=82=20Grzelak?= <michal.grzelak@intel.com>
Patches: 4
Reviewed: 2026-05-25T18:29:26.862760

---

This is a clean, straightforward 2-patch series that fixes documentation gaps in the DRM debug subsystem. Both patches are correct and address real issues: missing bit descriptions in `MODULE_PARM_DESC`, an out-of-date "enable all" bitmask comment, and a kernel-doc `@parameter` reference error. No functional code changes. The series is small, well-scoped, and ready to merge.

**Verdict: Looks good.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: drm/print: describe 6th & 9th bit of drm.debug
  2026-05-22 13:55 ` [PATCH v2 1/2] drm/print: describe 6th & 9th bit of drm.debug Michał Grzelak
  2026-05-22 19:55   ` Michał Grzelak
@ 2026-05-25  8:29   ` Claude Code Review Bot
  1 sibling, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  8:29 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

This patch adds the missing `Bit 6 (STATE)` and `Bit 9 (DRMRES)` entries to the `MODULE_PARM_DESC` string, and fixes the "enable all" comment from `0x1ff` to `0x3ff`.

**Correctness:** I verified against the enum `drm_debug_category` in `include/drm/drm_print.h:98-142` and the `DECLARE_DYNDBG_CLASSMAP` in `drivers/gpu/drm/drm_print.c:60-70`. There are 10 categories (bits 0–9):

| Bit | Enum | Present in MODULE_PARM_DESC? |
|-----|------|-----|
| 0–5 | CORE through VBL | Yes (already) |
| 6 | DRM_UT_STATE | **Missing → added by patch** |
| 7 | DRM_UT_LEASE | Yes (already) |
| 8 | DRM_UT_DP | Yes (already) |
| 9 | DRM_UT_DRMRES | **Missing → added by patch** |

The descriptions match the enum comments:
- `DRM_UT_STATE` → "Used for verbose atomic state debugging" → patch uses "STATE messages (atomic state code)" — consistent with the existing style.
- `DRM_UT_DRMRES` → "Used in the drm managed resources code" → patch uses "DRMRES messages (managed resources code)" — correct.

The bitmask fix from `0x1ff` (bits 0–8) to `0x3ff` (bits 0–9) is also correct, since 10 categories need 10 bits.

**Minor nit (non-blocking):** The subject says "6th & 9th bit" but the code adds "Bit 6" and "Bit 9" (0-indexed). "6th bit" in natural language would traditionally be bit 5. Consider "bit 6 & bit 9" in the subject for precision, though this is entirely cosmetic.

**Reviewed-by worthy:** Yes.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: drm/managed: fix drmm_add_action() kernel-doc
  2026-05-22 13:55 ` [PATCH v2 2/2] drm/managed: fix drmm_add_action() kernel-doc Michał Grzelak
@ 2026-05-25  8:29   ` Claude Code Review Bot
  0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  8:29 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

This patch fixes a kernel-doc `@parameter` reference error in the `drmm_add_action()` macro documentation.

**Before:**
```c
 * This function adds the @release action with optional parameter @data to the
```

**After:**
```c
 * This function adds the release @action with optional parameter @data to the
```

The macro's parameters are `dev`, `action`, and `data`. The old text used `@release`, which is not a parameter — kernel-doc would flag this as an undocumented/invalid reference. The fix swaps the `@` to correctly annotate `@action` (the actual parameter) and makes `release` a plain descriptive word. This is correct.

I confirmed by reading `include/drm/drm_managed.h:15-26` — the macro is:
```c
#define drmm_add_action(dev, action, data) \
	__drmm_add_action(dev, action, data, #action)
```

The parameter list is `@dev`, `@action`, `@data` — matching the corrected doc.

**Reviewed-by worthy:** Yes.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: drm/print: describe 6th & 9th bit of drm.debug
  2026-05-21 15:52 ` [PATCH v1 1/2] drm/print: describe 6th & 9th bit of drm.debug Michał Grzelak
@ 2026-05-25  9:48   ` Claude Code Review Bot
  0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  9:48 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good.** The patch correctly adds the two missing bit descriptions to `MODULE_PARM_DESC`. Verified against the kernel tree:

The enum `drm_debug_category` in `include/drm/drm_print.h:98-142` defines entries in order: `DRM_UT_CORE`(0), `DRM_UT_DRIVER`(1), `DRM_UT_KMS`(2), `DRM_UT_PRIME`(3), `DRM_UT_ATOMIC`(4), `DRM_UT_VBL`(5), `DRM_UT_STATE`(6), `DRM_UT_LEASE`(7), `DRM_UT_DP`(8), `DRM_UT_DRMRES`(9). The `DECLARE_DYNDBG_CLASSMAP` at `drm_print.c:60` confirms the same order. The existing `MODULE_PARM_DESC` was indeed missing bit 6 (`STATE`) and bit 9 (`DRMRES`).

The added lines:
```
+"\t\tBit 6 (0x40)  will enable STATE messages (atomic state code)\n"
...
+"\t\tBit 9 (0x200) will enable DRMRES messages (managed resources code)");
```

These match the enum doc: `DRM_UT_STATE` is "verbose atomic state debugging" and `DRM_UT_DRMRES` is "drm managed resources code". The descriptions are consistent.

The change from `0x1ff` to `0x3ff` in `include/drm/drm_print.h:90` is also correct: with 10 categories (bits 0-9), all-bits-set is `0x3ff`, not `0x1ff` (which only covers bits 0-8). This was already wrong before `DRM_UT_DRMRES` was added — the comment was stale.

**No issues found.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-25  9:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-22 13:55 [PATCH v2 0/2] fixes to DRM doc & parameter's print Michał Grzelak
2026-05-22 13:55 ` [PATCH v2 1/2] drm/print: describe 6th & 9th bit of drm.debug Michał Grzelak
2026-05-22 19:55   ` Michał Grzelak
2026-05-25  8:29   ` Claude review: " Claude Code Review Bot
2026-05-22 13:55 ` [PATCH v2 2/2] drm/managed: fix drmm_add_action() kernel-doc Michał Grzelak
2026-05-25  8:29   ` Claude review: " Claude Code Review Bot
2026-05-25  8:29 ` Claude review: fixes to DRM doc & parameter's print Claude Code Review Bot
  -- strict thread matches above, loose matches on Subject: below --
2026-05-21 15:52 [PATCH v1 0/2] " Michał Grzelak
2026-05-21 15:52 ` [PATCH v1 1/2] drm/print: describe 6th & 9th bit of drm.debug Michał Grzelak
2026-05-25  9:48   ` Claude review: " Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox