[PATCH v2 0/3] drm/imagination: Firmware handling of context reset notification

[PATCH v2 0/3] drm/imagination: Firmware handling of context reset notification

[Alexandru Dadu]

- patch 1: Adds missing context reset reasons.
- patch 2: Fixes the reset_reason from enum to u32 in the fwif.
- patch 3: Adds the implementation of the context reset notification.

Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
Changes in v2:
- enum rogue_context_reset_reason update moved to a separate commit;
- pvr_context_reset_notification() renamed to pvr_dump_context_reset_notification();
- added handling for PVR_FWIF_DM_TDM;
- update include order in pvr_dump.c;
- fixed enum type ambiguilty in fwif headers for the reset_reason struct field;
- small QOL and nits fixed.
- Link to v1: https://lore.kernel.org/r/20260305-b4-firmware-context-reset-notification-handling-v1-1-46e2ce8d46ef@imgtec.com

---
Alexandru Dadu (3):
      drm/imagination: Add missing rogue context reset reasons
      drm/imagination: Switch reset_reason fields from enum to u32
      drm/imagination: Implement handling of context reset notification

 drivers/gpu/drm/imagination/Makefile               |   1 +
 drivers/gpu/drm/imagination/pvr_ccb.c              |   5 +
 drivers/gpu/drm/imagination/pvr_dump.c             | 113 +++++++++++++++++++++
 drivers/gpu/drm/imagination/pvr_dump.h             |  17 ++++
 drivers/gpu/drm/imagination/pvr_rogue_fwif.h       |   8 +-
 .../gpu/drm/imagination/pvr_rogue_fwif_shared.h    |  18 +++-
 6 files changed, 159 insertions(+), 3 deletions(-)
---
base-commit: d2e20c8951e4bb5f4a828aed39813599980353b6
change-id: 20260305-b4-firmware-context-reset-notification-handling-694a1b5e6b8c

Best regards,
-- 
Alexandru Dadu <alexandru.dadu@imgtec.com>

[PATCH v2 1/3] drm/imagination: Add missing rogue context reset reasons

[Alexandru Dadu]

Update the context reset reason enum with the missing reset reasons in
the 6-11 value gap:
 - CDM Mission/safety checksum mismatch;
 - TRP checksum mismatch;
 - GPU ECC error (corrected, OK);
 - GPU ECC error (uncorrected, HWR);
 - FW ECC error (corrected, OK);
 - FW ECC error (uncorrected, ERR);

Co-authored-by: Sarah Walker <sarah.walker@imgtec.com>
Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h b/drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h
index 6c09c15bf9bd..f622553cdc11 100644
--- a/drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h
+++ b/drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h
@@ -236,6 +236,18 @@ enum rogue_context_reset_reason {
 	ROGUE_CONTEXT_RESET_REASON_INNOCENT_OVERRUNING = 4,
 	/* Forced reset to ensure scheduling requirements */
 	ROGUE_CONTEXT_RESET_REASON_HARD_CONTEXT_SWITCH = 5,
+	/* CDM Mission/safety checksum mismatch */
+	ROGUE_CONTEXT_RESET_REASON_WGP_CHECKSUM = 6,
+	/* TRP checksum mismatch */
+	ROGUE_CONTEXT_RESET_REASON_TRP_CHECKSUM = 7,
+	/* GPU ECC error (corrected, OK) */
+	ROGUE_CONTEXT_RESET_REASON_GPU_ECC_OK = 8,
+	/* GPU ECC error (uncorrected, HWR) */
+	ROGUE_CONTEXT_RESET_REASON_GPU_ECC_HWR = 9,
+	/* FW ECC error (corrected, OK) */
+	ROGUE_CONTEXT_RESET_REASON_FW_ECC_OK = 10,
+	/* FW ECC error (uncorrected, ERR) */
+	ROGUE_CONTEXT_RESET_REASON_FW_ECC_ERR = 11,
 	/* FW Safety watchdog triggered */
 	ROGUE_CONTEXT_RESET_REASON_FW_WATCHDOG = 12,
 	/* FW page fault (no HWR) */

-- 
2.43.0

Claude review: drm/imagination: Add missing rogue context reset reasons

[Claude Code Review Bot]

Patch Review

This patch fills in the gap (values 6–11) in `enum rogue_context_reset_reason`. The values and comments look correct and consistent with the existing entries.

**Looks good.** No issues found. The Co-authored-by tag for Sarah Walker is present.

One minor note: this patch does not carry a `Fixes:` tag, though it could arguably have one since the enum was incomplete from the initial FWIF headers commit. However, since these values weren't consumed by any code until patch 3, it's reasonable to leave it without one.

---

---
Generated by Claude Code Patch Reviewer

[PATCH v2 2/3] drm/imagination: Switch reset_reason fields from enum to u32

[Alexandru Dadu]

Update the reset_reason fwif structure fields from enum to u32 to remove
any ambiguity from the interface (enum is not a fixed size thus is unfit
for the purpose of the data type).

Fixes: a26f067feac1f ("drm/imagination: Add FWIF headers")

Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_rogue_fwif.h        | 8 ++++++--
 drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h | 6 +++++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_rogue_fwif.h b/drivers/gpu/drm/imagination/pvr_rogue_fwif.h
index 172886be4c82..5d590c4c2566 100644
--- a/drivers/gpu/drm/imagination/pvr_rogue_fwif.h
+++ b/drivers/gpu/drm/imagination/pvr_rogue_fwif.h
@@ -1347,8 +1347,12 @@ struct rogue_fwif_fwccb_cmd_freelists_reconstruction_data {
 struct rogue_fwif_fwccb_cmd_context_reset_data {
 	/* Context affected by the reset */
 	u32 server_common_context_id;
-	/* Reason for reset */
-	enum rogue_context_reset_reason reset_reason;
+	/*
+	 * Reason for reset
+	 * The valid values for reset_reason are the ones from
+	 * enum rogue_context_reset_reason
+	 */
+	u32 reset_reason;
 	/* Data Master affected by the reset */
 	u32 dm;
 	/* Job ref running at the time of reset */
diff --git a/drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h b/drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h
index f622553cdc11..869d904e3649 100644
--- a/drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h
+++ b/drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h
@@ -261,7 +261,11 @@ enum rogue_context_reset_reason {
 };
 
 struct rogue_context_reset_reason_data {
-	enum rogue_context_reset_reason reset_reason;
+	/*
+	 * The valid values for reset_reason are the ones from
+	 * enum rogue_context_reset_reason
+	 */
+	u32 reset_reason;
 	u32 reset_ext_job_ref;
 };
 

-- 
2.43.0

Claude review: drm/imagination: Switch reset_reason fields from enum to u32

[Claude Code Review Bot]

Patch Review

This patch changes `reset_reason` from `enum rogue_context_reset_reason` to `u32` in two structs that define firmware-shared data structures. The rationale is correct — `enum` size is compiler-dependent and unsuitable for firmware ABI structures.

The `Fixes:` tag referencing the original FWIF headers commit is appropriate.

**Minor observation:** The patch changes the field type in `rogue_fwif_fwccb_cmd_context_reset_data` and `rogue_context_reset_reason_data`, but there are likely existing users of `reset_reason_data.reset_reason` elsewhere in the driver that previously relied on the enum type. Let me verify this isn't a problem:

The existing code in `pvr_context.c` that reads `reset_reason` from `rogue_context_reset_reason_data` should be unaffected since `u32` is assignment-compatible with the previous enum values. No functional issues.

**Looks good.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v2 3/3] drm/imagination: Implement handling of context reset notification

[Alexandru Dadu]

The firmware will send the context reset notification message as
part of handling hardware recovery (HWR) events deecoding the message
and printing via drm_info(). This eliminates the "Unknown FWCCB command"
message that was previously printed.

Co-authored-by: Sarah Walker <sarah.walker@imgtec.com>
Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
 drivers/gpu/drm/imagination/Makefile   |   1 +
 drivers/gpu/drm/imagination/pvr_ccb.c  |   5 ++
 drivers/gpu/drm/imagination/pvr_dump.c | 113 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/imagination/pvr_dump.h |  17 +++++
 4 files changed, 136 insertions(+)

diff --git a/drivers/gpu/drm/imagination/Makefile b/drivers/gpu/drm/imagination/Makefile
index f5072f06b4c4..1222a14262e4 100644
--- a/drivers/gpu/drm/imagination/Makefile
+++ b/drivers/gpu/drm/imagination/Makefile
@@ -8,6 +8,7 @@ powervr-y := \
 	pvr_device.o \
 	pvr_device_info.o \
 	pvr_drv.o \
+	pvr_dump.o \
 	pvr_free_list.o \
 	pvr_fw.o \
 	pvr_fw_meta.o \
diff --git a/drivers/gpu/drm/imagination/pvr_ccb.c b/drivers/gpu/drm/imagination/pvr_ccb.c
index f89db5e3baa2..a04520e7efc0 100644
--- a/drivers/gpu/drm/imagination/pvr_ccb.c
+++ b/drivers/gpu/drm/imagination/pvr_ccb.c
@@ -4,6 +4,7 @@
 #include "pvr_ccb.h"
 #include "pvr_device.h"
 #include "pvr_drv.h"
+#include "pvr_dump.h"
 #include "pvr_free_list.h"
 #include "pvr_fw.h"
 #include "pvr_gem.h"
@@ -165,6 +166,10 @@ process_fwccb_command(struct pvr_device *pvr_dev, struct rogue_fwif_fwccb_cmd *c
 		 * suppress the "unknown" warning when receiving this command.
 		 */
 		break;
+	case ROGUE_FWIF_FWCCB_CMD_CONTEXT_RESET_NOTIFICATION:
+		pvr_dump_context_reset_notification(pvr_dev,
+					       &cmd->cmd_data.cmd_context_reset_notification);
+		break;
 
 	default:
 		drm_info(drm_dev, "Received unknown FWCCB command (type=%d)\n",
diff --git a/drivers/gpu/drm/imagination/pvr_dump.c b/drivers/gpu/drm/imagination/pvr_dump.c
new file mode 100644
index 000000000000..bb52eea8b63a
--- /dev/null
+++ b/drivers/gpu/drm/imagination/pvr_dump.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/* Copyright (c) 2026 Imagination Technologies Ltd. */
+
+#include "pvr_device.h"
+#include "pvr_dump.h"
+#include "pvr_rogue_fwif.h"
+
+#include <drm/drm_print.h>
+#include <linux/types.h>
+
+static const char *
+get_reset_reason_desc(enum rogue_context_reset_reason reason)
+{
+	switch (reason) {
+	case ROGUE_CONTEXT_RESET_REASON_NONE:
+		return "None";
+	case ROGUE_CONTEXT_RESET_REASON_GUILTY_LOCKUP:
+		return "Guilty lockup";
+	case ROGUE_CONTEXT_RESET_REASON_INNOCENT_LOCKUP:
+		return "Innocent lockup";
+	case ROGUE_CONTEXT_RESET_REASON_GUILTY_OVERRUNING:
+		return "Guilty overrunning";
+	case ROGUE_CONTEXT_RESET_REASON_INNOCENT_OVERRUNING:
+		return "Innocent overrunning";
+	case ROGUE_CONTEXT_RESET_REASON_HARD_CONTEXT_SWITCH:
+		return "Hard context switch";
+	case ROGUE_CONTEXT_RESET_REASON_WGP_CHECKSUM:
+		return "CDM Mission/safety checksum mismatch";
+	case ROGUE_CONTEXT_RESET_REASON_TRP_CHECKSUM:
+		return "TRP checksum mismatch";
+	case ROGUE_CONTEXT_RESET_REASON_GPU_ECC_OK:
+		return "GPU ECC error (corrected, OK)";
+	case ROGUE_CONTEXT_RESET_REASON_GPU_ECC_HWR:
+		return "GPU ECC error (uncorrected, HWR)";
+	case ROGUE_CONTEXT_RESET_REASON_FW_ECC_OK:
+		return "Firmware ECC error (corrected, OK)";
+	case ROGUE_CONTEXT_RESET_REASON_FW_ECC_ERR:
+		return "Firmware ECC error (uncorrected, ERR)";
+	case ROGUE_CONTEXT_RESET_REASON_FW_WATCHDOG:
+		return "Firmware watchdog";
+	case ROGUE_CONTEXT_RESET_REASON_FW_PAGEFAULT:
+		return "Firmware pagefault";
+	case ROGUE_CONTEXT_RESET_REASON_FW_EXEC_ERR:
+		return "Firmware execution error";
+	case ROGUE_CONTEXT_RESET_REASON_HOST_WDG_FW_ERR:
+		return "Host watchdog";
+	case ROGUE_CONTEXT_GEOM_OOM_DISABLED:
+		return "Geometry OOM disabled";
+
+	default:
+		return "Unknown";
+	}
+}
+
+static const char *
+get_dm_name(u32 dm)
+{
+	switch (dm) {
+	case PVR_FWIF_DM_GP:
+		return "General purpose";
+	/* PVR_FWIF_DM_TDM has the same index, but is discriminated by a device feature */
+	case PVR_FWIF_DM_2D:
+		return "2D or TDM";
+	case PVR_FWIF_DM_GEOM:
+		return "Geometry";
+	case PVR_FWIF_DM_FRAG:
+		return "Fragment";
+	case PVR_FWIF_DM_CDM:
+		return "Compute";
+	case PVR_FWIF_DM_RAY:
+		return "Raytracing";
+	case PVR_FWIF_DM_GEOM2:
+		return "Geometry 2";
+	case PVR_FWIF_DM_GEOM3:
+		return "Geometry 3";
+	case PVR_FWIF_DM_GEOM4:
+		return "Geometry 4";
+
+	default:
+		return "Unknown";
+	}
+}
+
+/**
+ * pvr_dump_context_reset_notification() - Handle context reset notification from FW
+ * @pvr_dev: Device pointer.
+ * @data: Data provided by FW.
+ *
+ * This will decode the data structure provided by FW and print the results via drm_info().
+ */
+void
+pvr_dump_context_reset_notification(struct pvr_device *pvr_dev,
+			       struct rogue_fwif_fwccb_cmd_context_reset_data *data)
+{
+	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
+
+	if (data->flags & ROGUE_FWIF_FWCCB_CMD_CONTEXT_RESET_FLAG_ALL_CTXS) {
+		drm_info(drm_dev, "Received context reset notification for all contexts\n");
+	} else {
+		drm_info(drm_dev, "Received context reset notification on context %u\n",
+			 data->server_common_context_id);
+	}
+
+	drm_info(drm_dev, "  Reset reason=%u (%s)\n", data->reset_reason,
+		 get_reset_reason_desc((enum rogue_context_reset_reason)data->reset_reason));
+	drm_info(drm_dev, "  Data Master=%u (%s)\n", data->dm, get_dm_name(data->dm));
+	drm_info(drm_dev, "  Job ref=%u\n", data->reset_job_ref);
+
+	if (data->flags & ROGUE_FWIF_FWCCB_CMD_CONTEXT_RESET_FLAG_PF) {
+		drm_info(drm_dev, "  Page fault occurred, fault address=%llx\n",
+			data->fault_address);
+	}
+}
diff --git a/drivers/gpu/drm/imagination/pvr_dump.h b/drivers/gpu/drm/imagination/pvr_dump.h
new file mode 100644
index 000000000000..1e3e68694f5f
--- /dev/null
+++ b/drivers/gpu/drm/imagination/pvr_dump.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+/* Copyright (c) 2026 Imagination Technologies Ltd. */
+
+#ifndef PVR_DUMP_H
+#define PVR_DUMP_H
+
+/* Forward declaration from pvr_device.h. */
+struct pvr_device;
+
+/* Forward declaration from pvr_rogue_fwif.h. */
+struct rogue_fwif_fwccb_cmd_context_reset_data;
+
+void
+pvr_dump_context_reset_notification(struct pvr_device *pvr_dev,
+			       struct rogue_fwif_fwccb_cmd_context_reset_data *data);
+
+#endif /* PVR_DUMP_H */

-- 
2.43.0

Claude review: drm/imagination: Implement handling of context reset notification

[Claude Code Review Bot]

Patch Review

This is the main patch. It adds a new `pvr_dump.c` / `pvr_dump.h` and hooks the `ROGUE_FWIF_FWCCB_CMD_CONTEXT_RESET_NOTIFICATION` case into `process_fwccb_command()`.

**Issues and observations:**

1. **Typo in commit message:** "deecoding" should be "decoding":
   > `part of handling hardware recovery (HWR) events deecoding the message`

2. **Missing Co-authored-by tag format:** The `Co-authored-by:` tag is present but there is no `Signed-off-by` from Sarah Walker. Kernel convention typically expects a `Signed-off-by` from all co-authors to certify DCO compliance.

3. **`get_dm_name()` comment about PVR_FWIF_DM_TDM:** The code has:
   ```c
   /* PVR_FWIF_DM_TDM has the same index, but is discriminated by a device feature */
   case PVR_FWIF_DM_2D:
       return "2D or TDM";
   ```
   Since `PVR_FWIF_DM_2D` and `PVR_FWIF_DM_TDM` both equal 1 (confirmed in `pvr_rogue_fwif_common.h:26,31`), you can't have separate `case` labels. The "2D or TDM" string is a reasonable compromise, but it could be improved by checking the device features at runtime (via `pvr_dev`) to print the correct name. The `pvr_device` is already passed to the caller. This is a minor nit though — the current approach is functional.

4. **Cast from u32 to enum in `get_reset_reason_desc` call:**
   ```c
   get_reset_reason_desc((enum rogue_context_reset_reason)data->reset_reason)
   ```
   This is fine functionally, but the `default: return "Unknown"` case in `get_reset_reason_desc()` properly handles unexpected values, which is good. However, this cast slightly contradicts the spirit of patch 2 which changed the field to `u32` precisely because enum sizes aren't fixed. Consider accepting a `u32` parameter in `get_reset_reason_desc()` instead, which would be more consistent. This is a minor style point.

5. **`pvr_dump.c` / `pvr_dump.h` naming:** The file name `pvr_dump` is quite generic. Right now it only contains context reset notification handling. If more dump-type functionality is expected, the name is fine. If this is specifically about FW CCB notification handling, a more specific name might be clearer. This is subjective and not blocking.

6. **Alignment of function parameter continuation in pvr_ccb.c:**
   ```c
   pvr_dump_context_reset_notification(pvr_dev,
                          &cmd->cmd_data.cmd_context_reset_notification);
   ```
   The continuation line alignment looks slightly off (tabs vs spaces mix), but this matches the surrounding code style in the file, so it's acceptable.

7. **Log level consideration:** All notifications are printed via `drm_info()`. For high-frequency HWR events this could be noisy. A rate-limited variant (`drm_info_ratelimited()`) or using `drm_dbg()` for the detail lines might be worth considering, but this is a design choice the maintainers can weigh in on.

**Overall:** The series is straightforward and correct. The main actionable items are the commit message typo and the missing `Signed-off-by` from the co-author.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/imagination: Firmware handling of context reset notification

[Claude Code Review Bot]

Overall Series Review

Subject: drm/imagination: Firmware handling of context reset notification
Author: Alexandru Dadu <alexandru.dadu@imgtec.com>
Patches: 4
Reviewed: 2026-03-13T14:16:57.757821

---

This is a clean, well-structured 3-patch series that adds firmware context reset notification handling to the PowerVR (Imagination) DRM driver. The patches are logically ordered: (1) fill in missing enum values, (2) fix the firmware interface type from enum to u32, (3) implement the actual notification handler. The code is straightforward and the series is in good shape overall. A few minor items are worth noting.

---
Generated by Claude Code Patch Reviewer