From c2369bc3abc8dbaec08178beaf33744765116856 Mon Sep 17 00:00:00 2001 From: UjinT34 <41110182+UjinT34@users.noreply.github.com> Date: Fri, 10 Jan 2025 21:38:51 +0300 Subject: [PATCH] drm: Avoid excessive atomic properties updates (#95) * do not set cursor planeProps unless smth changed * do not skip cursor state flag setting * drm: scan only cards and not outputs, safeguard against null renderer (#106) * drm: dont scan card outputs no need to scan for card[0-9]* and probe card0-eDP etc if they are kms, bootvga and rendernodes etc. skip the wildcard and remove a unused size_t variable. * drm: dont commit state if renderer is missing setting certain env vars to force egl implentations makes the render creation fail on the second gpu. instead of causing a coredump, safeguard commitState and let the monitor turn blank instead. * props: bump version to 0.5.0 * drm: Validate conn before dereference in CDRMAtomicRequest::commit() (#108) During startup, CDRMAtomicImpl::reset() may emit a call to method commit of a CDRMAtomicRequest instance with member "conn" uninitialized, leading to a segfault. Validate the the pointer before dereference it as a workaround. Fixes: 55ac962 ("DRM: preliminary atomic support") Closes: https://github.com/hyprwm/aquamarine/issues/107 Signed-off-by: Yao Zi * buffer: remove useless forward def * drm: clearer flow when rescanning connectors (#113) * consolidates into checkOutput for clearer flow when rescanning connectors * add error log * drm: allow multigpu blit from explicit to implicit (#114) * version: bump to 0.5.1 * flake.lock: update * flake.nix: gcc13 -> gcc14 (#118) * drm: udev scan only drm_minor, not connectors (#119) * drm: log errno set by drmModeAtomicCommit (#120) * drm: moved null check for renderer to shouldBlit() (#109) (#121) * drm: only fail INVALID format when enabled (#122) * flake.lock: update * drm: only clear buffers when fullReconfigure succeeds (#124) * core/drm: Add HDR Support (#112) * version: bump to 0.6.0 * drm: limit udev drm_minor to Linux after a132fa41be7e (#129) Not implemented by libudev-devd yet: [ERR] [AQ] drm: No gpus in scanGPUs. [ERR] [AQ] drm: Found no gpus to use, cannot continue [ERR] [AQ] DRM Backend failed * do not set cursor planeProps unless smth changed * test separate cursor commits * do not change hdr blob unless asked to * rebase * split atomic commit processing and move hdr & colorspace into modeset * fix wide color gamut flag & cleanup * remove unused debug var --------- Signed-off-by: Yao Zi Co-authored-by: Tom Englund Co-authored-by: Vaxry Co-authored-by: Ziyao Co-authored-by: Ikalco <73481042+ikalco@users.noreply.github.com> Co-authored-by: Mihai Fufezan Co-authored-by: Austin Horstman Co-authored-by: Richard Henninger <56615615+richen604@users.noreply.github.com> Co-authored-by: Jan Beich --- include/aquamarine/backend/DRM.hpp | 2 +- include/aquamarine/backend/drm/Atomic.hpp | 4 + include/aquamarine/output/Output.hpp | 3 + src/backend/drm/DRM.cpp | 7 +- src/backend/drm/impl/Atomic.cpp | 99 ++++++++++++++++------- src/backend/drm/impl/Legacy.cpp | 3 +- src/output/Output.cpp | 1 + 7 files changed, 89 insertions(+), 30 deletions(-) diff --git a/include/aquamarine/backend/DRM.hpp b/include/aquamarine/backend/DRM.hpp index 7c2ddaa..cc03904 100644 --- a/include/aquamarine/backend/DRM.hpp +++ b/include/aquamarine/backend/DRM.hpp @@ -263,7 +263,7 @@ namespace Aquamarine { bool gammad = false; bool degammad = false; bool ctmd = false; - bool hdrd = false; + bool hdrd = false; // true if hdr blob needs updating or clearing } atomic; void calculateMode(Hyprutils::Memory::CSharedPointer connector); diff --git a/include/aquamarine/backend/drm/Atomic.hpp b/include/aquamarine/backend/drm/Atomic.hpp index ce814ed..919729b 100644 --- a/include/aquamarine/backend/drm/Atomic.hpp +++ b/include/aquamarine/backend/drm/Atomic.hpp @@ -23,10 +23,14 @@ namespace Aquamarine { CDRMAtomicRequest(Hyprutils::Memory::CWeakPointer backend); ~CDRMAtomicRequest(); + void setConnector(Hyprutils::Memory::CSharedPointer connector); void addConnector(Hyprutils::Memory::CSharedPointer connector, SDRMConnectorCommitData& data); + void addConnectorModeset(Hyprutils::Memory::CSharedPointer connector, SDRMConnectorCommitData& data); + void addConnectorCursor(Hyprutils::Memory::CSharedPointer connector, SDRMConnectorCommitData& data); bool commit(uint32_t flagssss); void add(uint32_t id, uint32_t prop, uint64_t val); void planeProps(Hyprutils::Memory::CSharedPointer plane, Hyprutils::Memory::CSharedPointer fb, uint32_t crtc, Hyprutils::Math::Vector2D pos); + void planePropsPos(Hyprutils::Memory::CSharedPointer plane, Hyprutils::Math::Vector2D pos); void rollback(SDRMConnectorCommitData& data); void apply(SDRMConnectorCommitData& data); diff --git a/include/aquamarine/output/Output.hpp b/include/aquamarine/output/Output.hpp index 5775753..5f5301e 100644 --- a/include/aquamarine/output/Output.hpp +++ b/include/aquamarine/output/Output.hpp @@ -55,6 +55,9 @@ namespace Aquamarine { AQ_OUTPUT_STATE_CTM = (1 << 10), AQ_OUTPUT_STATE_HDR = (1 << 11), AQ_OUTPUT_STATE_DEGAMMA_LUT = (1 << 12), + AQ_OUTPUT_STATE_WCG = (1 << 13), + AQ_OUTPUT_STATE_CURSOR_SHAPE = (1 << 14), + AQ_OUTPUT_STATE_CURSOR_POS = (1 << 15), }; struct SInternalState { diff --git a/src/backend/drm/DRM.cpp b/src/backend/drm/DRM.cpp index ec3d751..816d1c2 100644 --- a/src/backend/drm/DRM.cpp +++ b/src/backend/drm/DRM.cpp @@ -1546,7 +1546,8 @@ bool Aquamarine::CDRMOutput::commitState(bool onlyTest) { // which may result in some glitches const bool NEEDS_RECONFIG = COMMITTED & (COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_ENABLED | COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_FORMAT | - COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_MODE); + COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_MODE | COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_HDR | + COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_WCG); const bool BLOCKING = NEEDS_RECONFIG || !(COMMITTED & COutputState::eOutputStateProperties::AQ_OUTPUT_STATE_BUFFER); @@ -1752,6 +1753,7 @@ bool Aquamarine::CDRMOutput::setCursor(SP buffer, const Vector2D& hotsp if (!connector->crtc) return false; + state->internalState.committed |= COutputState::AQ_OUTPUT_STATE_CURSOR_SHAPE; if (!buffer) setCursorVisible(false); else { @@ -1826,6 +1828,9 @@ bool Aquamarine::CDRMOutput::setCursor(SP buffer, const Vector2D& hotsp void Aquamarine::CDRMOutput::moveCursor(const Vector2D& coord, bool skipSchedule) { cursorPos = coord; // cursorVisible = true; + // if (!skipSchedule) + state->internalState.committed |= COutputState::AQ_OUTPUT_STATE_CURSOR_POS; + backend->impl->moveCursor(connector, skipSchedule); } diff --git a/src/backend/drm/impl/Atomic.cpp b/src/backend/drm/impl/Atomic.cpp index 285bba5..3eddc39 100644 --- a/src/backend/drm/impl/Atomic.cpp +++ b/src/backend/drm/impl/Atomic.cpp @@ -57,10 +57,10 @@ void Aquamarine::CDRMAtomicRequest::planeProps(Hyprutils::Memory::CSharedPointer return; } - TRACE(backend->log(AQ_LOG_TRACE, - std::format("atomic planeProps: prop blobs: src_x {}, src_y {}, src_w {}, src_h {}, crtc_w {}, crtc_h {}, fb_id {}, crtc_id {}, crtc_x {}, crtc_y {}", - plane->props.src_x, plane->props.src_y, plane->props.src_w, plane->props.src_h, plane->props.crtc_w, plane->props.crtc_h, plane->props.fb_id, - plane->props.crtc_id, plane->props.crtc_x, plane->props.crtc_y))); + TRACE( + backend->log(AQ_LOG_TRACE, + std::format("atomic planeProps: prop blobs: src_x {}, src_y {}, src_w {}, src_h {}, crtc_w {}, crtc_h {}, fb_id {}, crtc_id {}", plane->props.src_x, + plane->props.src_y, plane->props.src_w, plane->props.src_h, plane->props.crtc_w, plane->props.crtc_h, plane->props.fb_id, plane->props.crtc_id))); // src_ are 16.16 fixed point (lol) add(plane->id, plane->props.src_x, 0); @@ -71,10 +71,24 @@ void Aquamarine::CDRMAtomicRequest::planeProps(Hyprutils::Memory::CSharedPointer add(plane->id, plane->props.crtc_h, (uint32_t)fb->buffer->size.y); add(plane->id, plane->props.fb_id, fb->id); add(plane->id, plane->props.crtc_id, crtc); + planePropsPos(plane, pos); +} + +void Aquamarine::CDRMAtomicRequest::planePropsPos(Hyprutils::Memory::CSharedPointer plane, Hyprutils::Math::Vector2D pos) { + + if (failed) + return; + + TRACE(backend->log(AQ_LOG_TRACE, std::format("atomic planeProps: pos blobs: crtc_x {}, crtc_y {}", plane->props.crtc_x, plane->props.crtc_y))); + add(plane->id, plane->props.crtc_x, (uint64_t)pos.x); add(plane->id, plane->props.crtc_y, (uint64_t)pos.y); } +void Aquamarine::CDRMAtomicRequest::setConnector(Hyprutils::Memory::CSharedPointer connector) { + conn = connector; +} + void Aquamarine::CDRMAtomicRequest::addConnector(Hyprutils::Memory::CSharedPointer connector, SDRMConnectorCommitData& data) { const auto& STATE = connector->output->state->state(); const bool enable = STATE.enabled && data.mainFB; @@ -85,23 +99,17 @@ void Aquamarine::CDRMAtomicRequest::addConnector(Hyprutils::Memory::CSharedPoint TRACE(backend->log(AQ_LOG_TRACE, std::format("atomic addConnector values: CRTC {}, mode {}", enable ? connector->crtc->id : 0, data.atomic.modeBlob))); + conn = connector; + + addConnectorModeset(connector, data); + addConnectorCursor(connector, data); + add(connector->id, connector->props.crtc_id, enable ? connector->crtc->id : 0); - if (data.modeset) { - add(connector->crtc->id, connector->crtc->props.mode_id, data.atomic.modeBlob); - data.atomic.blobbed = true; - } - - if (data.modeset && enable && connector->props.link_status) - add(connector->id, connector->props.link_status, DRM_MODE_LINK_STATUS_GOOD); - // TODO: allow to send aq a content type, maybe? Wayland has a protocol for this. if (enable && connector->props.content_type) add(connector->id, connector->props.content_type, DRM_MODE_CONTENT_TYPE_GRAPHICS); - if (data.modeset && enable && connector->props.max_bpc && connector->maxBpcBounds.at(1)) - add(connector->id, connector->props.max_bpc, 8); // FIXME: this isnt always 8 - add(connector->crtc->id, connector->crtc->props.active, enable); if (enable) { @@ -133,21 +141,57 @@ void Aquamarine::CDRMAtomicRequest::addConnector(Hyprutils::Memory::CSharedPoint if (connector->crtc->primary->props.fb_damage_clips) add(connector->crtc->primary->id, connector->crtc->primary->props.fb_damage_clips, data.atomic.fbDamage); - - if (connector->crtc->cursor) { - if (!connector->output->cursorVisible) - planeProps(connector->crtc->cursor, nullptr, 0, {}); - else - planeProps(connector->crtc->cursor, data.cursorFB, connector->crtc->id, connector->output->cursorPos - connector->output->cursorHotspot); - } - } else { planeProps(connector->crtc->primary, nullptr, 0, {}); - if (connector->crtc->cursor) - planeProps(connector->crtc->cursor, nullptr, 0, {}); } +} - conn = connector; +void Aquamarine::CDRMAtomicRequest::addConnectorModeset(Hyprutils::Memory::CSharedPointer connector, SDRMConnectorCommitData& data) { + if (!data.modeset) + return; + + const auto& STATE = connector->output->state->state(); + const bool enable = STATE.enabled && data.mainFB; + + add(connector->crtc->id, connector->crtc->props.mode_id, data.atomic.modeBlob); + data.atomic.blobbed = true; + + if (!enable) + return; + + if (connector->props.link_status) + add(connector->id, connector->props.link_status, DRM_MODE_LINK_STATUS_GOOD); + + if (connector->props.max_bpc && connector->maxBpcBounds.at(1)) + add(connector->id, connector->props.max_bpc, 8); // FIXME: this isnt always 8 + + if (connector->props.Colorspace && connector->colorspace.BT2020_RGB) + add(connector->id, connector->props.Colorspace, STATE.wideColorGamut ? connector->colorspace.BT2020_RGB : connector->colorspace.Default); + + if (connector->props.hdr_output_metadata && data.atomic.hdrd) + add(connector->id, connector->props.hdr_output_metadata, data.atomic.hdrBlob); +} + +void Aquamarine::CDRMAtomicRequest::addConnectorCursor(Hyprutils::Memory::CSharedPointer connector, SDRMConnectorCommitData& data) { + if (!connector->crtc->cursor) + return; + + const auto& STATE = connector->output->state->state(); + const bool enable = STATE.enabled && data.mainFB; + + if (enable) { + if (STATE.committed & COutputState::AQ_OUTPUT_STATE_CURSOR_SHAPE || STATE.committed & COutputState::AQ_OUTPUT_STATE_CURSOR_POS) { + TRACE(backend->log(AQ_LOG_TRACE, STATE.committed & COutputState::AQ_OUTPUT_STATE_CURSOR_SHAPE ? "atomic addConnector cursor shape" : "atomic addConnector cursor pos")); + if (STATE.committed & COutputState::AQ_OUTPUT_STATE_CURSOR_SHAPE) { + if (!connector->output->cursorVisible) + planeProps(connector->crtc->cursor, nullptr, 0, {}); + else + planeProps(connector->crtc->cursor, data.cursorFB, connector->crtc->id, connector->output->cursorPos - connector->output->cursorHotspot); + } else if (connector->output->cursorVisible) + planePropsPos(connector->crtc->cursor, connector->output->cursorPos - connector->output->cursorHotspot); + } + } else + planeProps(connector->crtc->cursor, nullptr, 0, {}); } bool Aquamarine::CDRMAtomicRequest::commit(uint32_t flagssss) { @@ -320,10 +364,11 @@ bool Aquamarine::CDRMAtomicImpl::prepareConnector(Hyprutils::Memory::CSharedPoin else { if (!data.hdrMetadata->hdmi_metadata_type1.eotf) { data.atomic.hdrBlob = 0; - data.atomic.hdrd = false; + data.atomic.hdrd = true; } else if (drmModeCreatePropertyBlob(connector->backend->gpu->fd, &data.hdrMetadata.value(), sizeof(hdr_output_metadata), &data.atomic.hdrBlob)) { connector->backend->backend->log(AQ_LOG_ERROR, "atomic drm: failed to create a hdr metadata blob"); data.atomic.hdrBlob = 0; + data.atomic.hdrd = false; } else { data.atomic.hdrd = true; TRACE(connector->backend->backend->log( diff --git a/src/backend/drm/impl/Legacy.cpp b/src/backend/drm/impl/Legacy.cpp index 87c11f6..6dac7e5 100644 --- a/src/backend/drm/impl/Legacy.cpp +++ b/src/backend/drm/impl/Legacy.cpp @@ -85,7 +85,8 @@ bool Aquamarine::CDRMLegacyImpl::commitInternal(Hyprutils::Memory::CSharedPointe // TODO: gamma - if (data.cursorFB && connector->crtc->cursor && connector->output->cursorVisible && enable) { + if (data.cursorFB && connector->crtc->cursor && connector->output->cursorVisible && enable && + (STATE.committed & COutputState::AQ_OUTPUT_STATE_CURSOR_SHAPE || STATE.committed & COutputState::AQ_OUTPUT_STATE_CURSOR_POS)) { uint32_t boHandle = 0; auto attrs = data.cursorFB->buffer->dmabuf(); diff --git a/src/output/Output.cpp b/src/output/Output.cpp index 5be02d2..9aa34cf 100644 --- a/src/output/Output.cpp +++ b/src/output/Output.cpp @@ -130,6 +130,7 @@ void Aquamarine::COutputState::setCTM(const Hyprutils::Math::Mat3x3& ctm) { void Aquamarine::COutputState::setWideColorGamut(bool wcg) { internalState.wideColorGamut = wcg; + internalState.committed |= AQ_OUTPUT_STATE_WCG; } void Aquamarine::COutputState::setHDRMetadata(const hdr_output_metadata& metadata) {