From 51f86caac39d94955540d4dc5afeb6d62131c575 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Sun, 19 Apr 2026 23:17:17 -0500 Subject: [PATCH] Fix assert on exit using KMSDRM on AMDGPU AMDGPU disallows commits that would leave a CRTC active without the primary plane enabled. Instead of disabling the primary plane, simply restore the original state of the plane to avoid this. --- app/streaming/video/ffmpeg-renderers/drm.cpp | 11 ++--- app/streaming/video/ffmpeg-renderers/drm.h | 47 ++++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/app/streaming/video/ffmpeg-renderers/drm.cpp b/app/streaming/video/ffmpeg-renderers/drm.cpp index d5d8454a..6758ac71 100644 --- a/app/streaming/video/ffmpeg-renderers/drm.cpp +++ b/app/streaming/video/ffmpeg-renderers/drm.cpp @@ -403,10 +403,10 @@ void DrmRenderer::cleanupRenderContext() // Ensure we're out of HDR mode setHdrMode(false); - // Deactivate all planes - m_PropSetter.disablePlane(m_VideoPlane); + // Restore active planes that we took over + m_PropSetter.restorePlane(m_VideoPlane); for (int i = 0; i < Overlay::OverlayMax; i++) { - m_PropSetter.disablePlane(m_OverlayPlanes[i]); + m_PropSetter.restorePlane(m_OverlayPlanes[i]); } // Revert our changes from prepareToRender() @@ -428,10 +428,7 @@ void DrmRenderer::cleanupRenderContext() } } for (auto &plane : m_UnusedActivePlanes) { - SDL_LogInfo(SDL_LOG_CATEGORY_APPLICATION, - "Restoring previously active plane: %u", - plane.second.objectId()); - m_PropSetter.restoreToInitial(plane.second); + m_PropSetter.restorePlane(plane.second); } m_PropSetter.apply(); diff --git a/app/streaming/video/ffmpeg-renderers/drm.h b/app/streaming/video/ffmpeg-renderers/drm.h index dcc3a752..06e876d9 100644 --- a/app/streaming/video/ffmpeg-renderers/drm.h +++ b/app/streaming/video/ffmpeg-renderers/drm.h @@ -466,6 +466,53 @@ class DrmRenderer : public IFFmpegRenderer { } } + void restorePlane(const DrmPropertyMap& plane) { + if (!plane.isValid()) { + return; + } + + if (isAtomic() && plane.property("FB_ID")->initialValue() != 0) { + SDL_LogInfo(SDL_LOG_CATEGORY_APPLICATION, + "Restoring previously active plane: %u", + plane.objectId()); + + { + std::lock_guard lg { m_Lock }; + auto &pb = m_PlaneBuffers[plane.objectId()]; + + // Free any pending buffers first + if (pb.pendingFbId) { + drmModeRmFB(m_Fd, pb.pendingFbId); + pb.pendingFbId = 0; + } + if (pb.pendingDumbBuffer) { + struct drm_mode_destroy_dumb destroyBuf = {}; + destroyBuf.handle = pb.pendingDumbBuffer; + drmIoctl(m_Fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroyBuf); + pb.pendingDumbBuffer = 0; + } + + // Since we restore the FB_ID, remember that we need to swap + // pending buffers to current after committing in apply(). + // This would normally be done in flipPlane(), but we can't + // use that here as it takes ownership of the FB. + // + // Since we cleared our pending buffers, the result is that + // apply() will free the current buffers and believe that no + // buffers are currently set. This will ensure it doesn't + // later free the restored FB_ID (which would happen if we + // used flipPlane() normally). + pb.modified = true; + } + + // Restore the old plane properties and FB_ID + restoreToInitial(plane); + } + else { + disablePlane(plane); + } + } + // The damage rect is relative to the FB void damagePlane(const DrmPropertyMap& plane, const SDL_Rect& rect) { SDL_assert(m_Atomic);