On 2/12/2026 5:08 PM, Hamza Mahfooz wrote:
There should be a mechanism for drivers to respond to flip_done
timeouts. Since, as it stands it is possible for the display to stall
indefinitely, necessitating a hard reset. So, introduce a new mechanism
that tries various methods of recovery with increasing aggression, in
the following order:
1. Force a full modeset (have the compositor reprogram the state from
scratch).
2. As a last resort, have the driver attempt a vendor specific reset
(which they can do by reading the return value of
drm_atomic_helper_wait_for_flip_done()).
Since you were able to (relatively) reliably reproduce a problem in
amdgpu, how far in your iterative flow did you get? Did you manage to
need the vendor specific handling? And presumably that helped?
Signed-off-by: Hamza Mahfooz <[email protected]>
---
v2: new to the series
v3: get rid of page_flip_timeout() and have
drm_atomic_helper_wait_for_flip_done() return a error.
---
drivers/gpu/drm/drm_atomic_helper.c | 45 +++++++++++++++++++++++++----
include/drm/drm_atomic_helper.h | 4 +--
include/drm/drm_device.h | 24 +++++++++++++++
3 files changed, 66 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 5840e9cc6f66..6ae1234b9e20 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -42,6 +42,7 @@
#include <drm/drm_gem_atomic_helper.h>
#include <drm/drm_panic.h>
#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
#include <drm/drm_self_refresh_helper.h>
#include <drm/drm_vblank.h>
#include <drm/drm_writeback.h>
@@ -1864,11 +1865,15 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
*
* This requires that drivers use the nonblocking commit tracking support
* initialized using drm_atomic_helper_setup_commit().
+ *
+ * Returns:
+ * -ETIMEDOUT to indicate that drivers can attempt a vendor reset, 0 otherwise.
*/
-void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev,
- struct drm_atomic_state *state)
+int drm_atomic_helper_wait_for_flip_done(struct drm_device *dev,
+ struct drm_atomic_state *state)
{
struct drm_crtc *crtc;
+ int ret = 0;
int i;
for (i = 0; i < dev->mode_config.num_crtc; i++) {
@@ -1881,13 +1886,43 @@ void drm_atomic_helper_wait_for_flip_done(struct
drm_device *dev,
continue;
ret = wait_for_completion_timeout(&commit->flip_done, 10 * HZ);
- if (ret == 0)
- drm_err(dev, "[CRTC:%d:%s] flip_done timed out\n",
- crtc->base.id, crtc->name);
+ if (!ret) {
+ switch (dev->reset_phase) {
+ case DRM_KMS_RESET_NONE:
+ drm_err(dev, "[CRTC:%d:%s] flip_done timed
out\n",
+ crtc->base.id, crtc->name);
+ dev->reset_phase = DRM_KMS_RESET_FORCE_MODESET;
+ drm_kms_helper_hotplug_event(dev);
+ break;
Since you're iterating multiple CRTCs if you manage to recover from one
with this call shouldn't you keep iterating the rest?
+ case DRM_KMS_RESET_FORCE_MODESET:
+ drm_err(dev, "[CRTC:%d:%s] force full modeset
failed\n",
+ crtc->base.id, crtc->name);
+ dev->reset_phase = DRM_KMS_RESET_VENDOR;
+ ret = -ETIMEDOUT;
+ break;
+ case DRM_KMS_RESET_VENDOR:
+ drm_err(dev, "[CRTC:%d:%s] KMS recovery
failed!\n",
+ crtc->base.id, crtc->name);
+ dev->reset_phase = DRM_KMS_RESET_GIVE_UP;
+ break;
+ default:
+ break;
+ }
+
+ goto exit;
+ }
+ }
+
+ if (dev->reset_phase) {
+ drm_info(dev, "KMS recovery succeeded!\n");
+ dev->reset_phase = DRM_KMS_RESET_NONE;
}
+exit:
if (state->fake_commit)
complete_all(&state->fake_commit->flip_done);
+
+ return ret;
}
EXPORT_SYMBOL(drm_atomic_helper_wait_for_flip_done);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 53382fe93537..298c8dff3993 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -79,8 +79,8 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
void drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
struct drm_atomic_state *old_state);
-void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev,
- struct drm_atomic_state *old_state);
+int drm_atomic_helper_wait_for_flip_done(struct drm_device *dev,
+ struct drm_atomic_state *old_state);
void
drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index bc78fb77cc27..1244d7527e7b 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -66,6 +66,23 @@ enum switch_power_state {
DRM_SWITCH_POWER_DYNAMIC_OFF = 3,
};
+/**
+ * enum drm_kms_reset_phase - reset phase of drm device
+ */
+enum drm_kms_reset_phase {
+ /** @DRM_KMS_RESET_NONE: Not currently attempting recovery */
+ DRM_KMS_RESET_NONE,
+
+ /** @DRM_KMS_RESET_FORCE_MODESET: Force a full modeset */
+ DRM_KMS_RESET_FORCE_MODESET,
+
+ /** @DRM_KMS_RESET_VENDOR: Attempt a vendor reset */
+ DRM_KMS_RESET_VENDOR,
+
+ /** @DRM_KMS_RESET_GIVE_UP: All recovery methods failed */
+ DRM_KMS_RESET_GIVE_UP,
+};
+
/**
* struct drm_device - DRM device structure
*
@@ -375,6 +392,13 @@ struct drm_device {
* Root directory for debugfs files.
*/
struct dentry *debugfs_root;
+
+ /**
+ * @reset_phase:
+ *
+ * Reset phase that the device is in.
+ */
+ enum drm_kms_reset_phase reset_phase;
};
void drm_dev_set_dma_dev(struct drm_device *dev, struct device *dma_dev);
