public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/amdgpu: amdgpu{_reset}.h: fix all kernel-doc warnings
@ 2026-03-05  0:55 Randy Dunlap
  2026-03-05  3:13 ` Claude review: " Claude Code Review Bot
  2026-03-05  3:13 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Randy Dunlap @ 2026-03-05  0:55 UTC (permalink / raw)
  To: dri-devel
  Cc: Randy Dunlap, Alex Deucher, Christian König, amd-gfx,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter

Fix all kernel-doc warnings in amdgpu.h and amdgpu_reset.h:
- Use the struct keyword for kernel-doc struct comments.
- Use the correct enum names in enum amd_reset_method.

This eliminates these warnings:

Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:477 cannot understand
 function prototype: 'struct amdgpu_wb'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Enum value
 'AMD_RESET_METHOD_LEGACY' not described in enum 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Enum value
 'AMD_RESET_METHOD_MODE0' not described in enum 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Enum value
 'AMD_RESET_METHOD_MODE1' not described in enum 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Enum value
 'AMD_RESET_METHOD_MODE2' not described in enum 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Enum value
 'AMD_RESET_METHOD_LINK' not described in enum 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Enum value
 'AMD_RESET_METHOD_BACO' not described in enum 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Enum value
 'AMD_RESET_METHOD_PCI' not described in enum 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Enum value
 'AMD_RESET_METHOD_ON_INIT' not described in enum 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Excess enum value
 '@AMD_RESET_LEGACY' description in 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Excess enum value
 '@AMD_RESET_MODE0' description in 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Excess enum value
 '@AMD_RESET_MODE1' description in 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Excess enum value
 '@AMD_RESET_MODE2' description in 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Excess enum value
 '@AMD_RESET_LINK' description in 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Excess enum value
 '@AMD_RESET_BACO' description in 'amd_reset_method'
Warning: drivers/gpu/drm/amd/amdgpu/amdgpu.h:576 Excess enum value
 '@AMD_RESET_PCI' description in 'amd_reset_method'

Also move the enum to amdgpu_reset.h and eventually only forward declare
it in amdgpu.h. (Christian)
I moved the enum to amdgpu_reset.h and then #included amdgpu_reset.h
in amdgpu.h. The simpler method causes build errors.

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>

 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |   42 ------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h |   69 +++++++++++++++-----
 2 files changed, 58 insertions(+), 53 deletions(-)

--- linux-next-20260304.orig/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ linux-next-20260304/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -463,7 +463,7 @@ int amdgpu_file_to_fpriv(struct file *fi
 #define AMDGPU_MAX_WB 1024	/* Reserve at most 1024 WB slots for amdgpu-owned rings. */
 
 /**
- * amdgpu_wb - This struct is used for small GPU memory allocation.
+ * struct amdgpu_wb - This struct is used for small GPU memory allocation.
  *
  * This struct is used to allocate a small amount of GPU memory that can be
  * used to shadow certain states into the memory. This is especially useful for
@@ -537,44 +537,6 @@ struct amdgpu_allowed_register_entry {
 	bool grbm_indexed;
 };
 
-/**
- * enum amd_reset_method - Methods for resetting AMD GPU devices
- *
- * @AMD_RESET_METHOD_NONE: The device will not be reset.
- * @AMD_RESET_LEGACY: Method reserved for SI, CIK and VI ASICs.
- * @AMD_RESET_MODE0: Reset the entire ASIC. Not currently available for the
- *                   any device.
- * @AMD_RESET_MODE1: Resets all IP blocks on the ASIC (SDMA, GFX, VCN, etc.)
- *                   individually. Suitable only for some discrete GPU, not
- *                   available for all ASICs.
- * @AMD_RESET_MODE2: Resets a lesser level of IPs compared to MODE1. Which IPs
- *                   are reset depends on the ASIC. Notably doesn't reset IPs
- *                   shared with the CPU on APUs or the memory controllers (so
- *                   VRAM is not lost). Not available on all ASICs.
- * @AMD_RESET_LINK: Triggers SW-UP link reset on other GPUs
- * @AMD_RESET_BACO: BACO (Bus Alive, Chip Off) method powers off and on the card
- *                  but without powering off the PCI bus. Suitable only for
- *                  discrete GPUs.
- * @AMD_RESET_PCI: Does a full bus reset using core Linux subsystem PCI reset
- *                 and does a secondary bus reset or FLR, depending on what the
- *                 underlying hardware supports.
- *
- * Methods available for AMD GPU driver for resetting the device. Not all
- * methods are suitable for every device. User can override the method using
- * module parameter `reset_method`.
- */
-enum amd_reset_method {
-	AMD_RESET_METHOD_NONE = -1,
-	AMD_RESET_METHOD_LEGACY = 0,
-	AMD_RESET_METHOD_MODE0,
-	AMD_RESET_METHOD_MODE1,
-	AMD_RESET_METHOD_MODE2,
-	AMD_RESET_METHOD_LINK,
-	AMD_RESET_METHOD_BACO,
-	AMD_RESET_METHOD_PCI,
-	AMD_RESET_METHOD_ON_INIT,
-};
-
 struct amdgpu_video_codec_info {
 	u32 codec_type;
 	u32 max_width;
@@ -1371,6 +1333,8 @@ int emu_soc_asic_init(struct amdgpu_devi
 #define RBIOS16(i) (RBIOS8(i) | (RBIOS8((i)+1) << 8))
 #define RBIOS32(i) ((RBIOS16(i)) | (RBIOS16((i)+2) << 16))
 
+#include "amdgpu_reset.h"
+
 /*
  * ASICs macro.
  */
--- linux-next-20260304.orig/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
+++ linux-next-20260304/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
@@ -46,6 +46,47 @@ enum AMDGPU_RESET_SRCS {
 	AMDGPU_RESET_SRC_USERQ,
 };
 
+/**
+ * enum amd_reset_method - Methods for resetting AMD GPU devices
+ *
+ * @AMD_RESET_METHOD_NONE: The device will not be reset.
+ * @AMD_RESET_METHOD_LEGACY: Method reserved for SI, CIK and VI ASICs.
+ * @AMD_RESET_METHOD_MODE0: Reset the entire ASIC. Not currently available for
+ *                          the any device.
+ * @AMD_RESET_METHOD_MODE1: Resets all IP blocks on the ASIC (SDMA, GFX, VCN,
+ *                   etc.) individually. Suitable only for some discrete GPU,
+ *                   not available for all ASICs.
+ * @AMD_RESET_METHOD_MODE2: Resets a lesser level of IPs compared to MODE1.
+ *                   Which IPs are reset depends on the ASIC. Notably doesn't
+ *                   reset IPs shared with the CPU on APUs or the memory
+ *                   controllers (so VRAM is not lost). Not available on all
+ *                   ASICs.
+ * @AMD_RESET_METHOD_LINK: Triggers SW-UP link reset on other GPUs
+ * @AMD_RESET_METHOD_BACO: BACO (Bus Alive, Chip Off) method powers off and on
+ *                   the card but without powering off the PCI bus. Suitable
+ *                   only for discrete GPUs.
+ * @AMD_RESET_METHOD_PCI: Does a full bus reset using core Linux subsystem
+ *                   PCI reset and does a secondary bus reset or FLR,
+ *                   depending on what the underlying hardware supports.
+ * @AMD_RESET_METHOD_ON_INIT: Does a device reset during the driver init
+ *                   sequence.
+ *
+ * Methods available for AMD GPU driver for resetting the device. Not all
+ * methods are suitable for every device. User can override the method using
+ * module parameter `reset_method`.
+ */
+enum amd_reset_method {
+	AMD_RESET_METHOD_NONE = -1,
+	AMD_RESET_METHOD_LEGACY = 0,
+	AMD_RESET_METHOD_MODE0,
+	AMD_RESET_METHOD_MODE1,
+	AMD_RESET_METHOD_MODE2,
+	AMD_RESET_METHOD_LINK,
+	AMD_RESET_METHOD_BACO,
+	AMD_RESET_METHOD_PCI,
+	AMD_RESET_METHOD_ON_INIT,
+};
+
 struct amdgpu_reset_context {
 	enum amd_reset_method method;
 	struct amdgpu_device *reset_req_dev;
@@ -56,6 +97,20 @@ struct amdgpu_reset_context {
 	enum AMDGPU_RESET_SRCS src;
 };
 
+struct amdgpu_reset_control {
+	void *handle;
+	struct work_struct reset_work;
+	struct mutex reset_lock;
+	struct amdgpu_reset_handler *(
+		*reset_handlers)[AMDGPU_RESET_MAX_HANDLERS];
+	atomic_t in_reset;
+	enum amd_reset_method active_reset;
+	struct amdgpu_reset_handler *(*get_reset_handler)(
+		struct amdgpu_reset_control *reset_ctl,
+		struct amdgpu_reset_context *context);
+	void (*async_reset)(struct work_struct *work);
+};
+
 struct amdgpu_reset_handler {
 	enum amd_reset_method reset_method;
 	int (*prepare_env)(struct amdgpu_reset_control *reset_ctl,
@@ -72,20 +127,6 @@ struct amdgpu_reset_handler {
 	int (*do_reset)(struct amdgpu_device *adev);
 };
 
-struct amdgpu_reset_control {
-	void *handle;
-	struct work_struct reset_work;
-	struct mutex reset_lock;
-	struct amdgpu_reset_handler *(
-		*reset_handlers)[AMDGPU_RESET_MAX_HANDLERS];
-	atomic_t in_reset;
-	enum amd_reset_method active_reset;
-	struct amdgpu_reset_handler *(*get_reset_handler)(
-		struct amdgpu_reset_control *reset_ctl,
-		struct amdgpu_reset_context *context);
-	void (*async_reset)(struct work_struct *work);
-};
-
 
 enum amdgpu_reset_domain_type {
 	SINGLE_DEVICE,

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

* Claude review: drm/amdgpu: amdgpu{_reset}.h: fix all kernel-doc warnings
  2026-03-05  0:55 [PATCH v2] drm/amdgpu: amdgpu{_reset}.h: fix all kernel-doc warnings Randy Dunlap
  2026-03-05  3:13 ` Claude review: " Claude Code Review Bot
@ 2026-03-05  3:13 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-05  3:13 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/amdgpu: amdgpu{_reset}.h: fix all kernel-doc warnings
Author: Randy Dunlap <rdunlap@infradead.org>
Patches: 1
Reviewed: 2026-03-05T13:13:38.346272

---

This is a single patch (v2) that fixes kernel-doc warnings in `amdgpu.h` and `amdgpu_reset.h`. It addresses three things: (1) adding the missing `struct` keyword to the `amdgpu_wb` doc comment, (2) updating enum member names in the `amd_reset_method` doc to match the actual code (`AMD_RESET_METHOD_*` instead of `AMD_RESET_*`), and (3) moving the `enum amd_reset_method` definition from `amdgpu.h` to `amdgpu_reset.h` per reviewer feedback. The doc fixes are correct, but the include placement creates a problematic ordering issue, and there's an unexplained struct reorder.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/amdgpu: amdgpu{_reset}.h: fix all kernel-doc warnings
  2026-03-05  0:55 [PATCH v2] drm/amdgpu: amdgpu{_reset}.h: fix all kernel-doc warnings Randy Dunlap
@ 2026-03-05  3:13 ` Claude Code Review Bot
  2026-03-05  3:13 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-05  3:13 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Doc fix for `amdgpu_wb` — Correct:**

```c
- * amdgpu_wb - This struct is used for small GPU memory allocation.
+ * struct amdgpu_wb - This struct is used for small GPU memory allocation.
```

Straightforward and correct. The `struct` keyword is required by kernel-doc.

**Enum doc name fixes — Correct:**

The doc comments now use `@AMD_RESET_METHOD_LEGACY`, `@AMD_RESET_METHOD_MODE0`, etc., matching the actual enum member names. Previously the docs used shortened names (`@AMD_RESET_LEGACY`, `@AMD_RESET_MODE0`) that didn't match. The new `@AMD_RESET_METHOD_ON_INIT` description is also a good addition — it was previously undocumented.

**Circular include / forward reference issue — Problem:**

`amdgpu_reset.h` includes `amdgpu.h` at line 27:
```c
#include "amdgpu.h"
```

The patch adds `#include "amdgpu_reset.h"` near line 1335 of `amdgpu.h`:
```c
+#include "amdgpu_reset.h"
```

But `enum amd_reset_method` is used at `amdgpu.h:572`:
```c
enum amd_reset_method (*reset_method)(struct amdgpu_device *adev);
```

When a compilation unit includes `amdgpu.h` first, the compiler reaches line 572 (the `reset_method` function pointer using `enum amd_reset_method`) before the `#include "amdgpu_reset.h"` at line 1335 where the enum is now defined. The enum type is used ~760 lines before it becomes visible.

GCC may tolerate this as a GNU extension (implicit forward declaration of the enum), but it's not clean C and may produce warnings with stricter compilers. A better approach would be to either:
- Place the `#include "amdgpu_reset.h"` before the struct that uses the enum (but this requires care with the circular dependency), or
- Add a forward declaration of the enum before its first use, or
- Break the circular dependency by having `amdgpu_reset.h` not include `amdgpu.h` and instead use forward declarations for the types it needs.

**Unexplained struct reorder in `amdgpu_reset.h`:**

The patch moves `struct amdgpu_reset_control` above `struct amdgpu_reset_handler`. This reordering is not mentioned in the commit message and doesn't fix any kernel-doc warning. Both structs reference each other only through pointers, so the order doesn't matter for compilation, but the change adds unnecessary noise to the diff. If there's a reason for this move, it should be documented in the commit message.

**Minor nit in comment text:**

The MODE0 description still has "the any device" (carried over from the original):
```
 * @AMD_RESET_METHOD_MODE0: Reset the entire ASIC. Not currently available for
 *                          the any device.
```
Should be "any device" (drop the extra "the"). Since this patch is already touching these comments, it would be a good opportunity to fix this typo.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-03-05  3:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-05  0:55 [PATCH v2] drm/amdgpu: amdgpu{_reset}.h: fix all kernel-doc warnings Randy Dunlap
2026-03-05  3:13 ` Claude review: " Claude Code Review Bot
2026-03-05  3:13 ` 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