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 <ziyao@disroot.org> * 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 a132fa41be (#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 <ziyao@disroot.org> Co-authored-by: Tom Englund <tomenglund26@gmail.com> Co-authored-by: Vaxry <vaxry@vaxry.net> Co-authored-by: Ziyao <ziyao@disroot.org> Co-authored-by: Ikalco <73481042+ikalco@users.noreply.github.com> Co-authored-by: Mihai Fufezan <mihai@fufexan.net> Co-authored-by: Austin Horstman <khaneliman12@gmail.com> Co-authored-by: Richard Henninger <56615615+richen604@users.noreply.github.com> Co-authored-by: Jan Beich <jbeich@FreeBSD.org>
2025-01-26 06:49:48 +01:00 · 2025-01-10 21:38:51 +03:00 · 2025-01-10 21:38:51 +03:00 · c2369bc3ab
commit c2369bc3ab
parent 5bc315ed56
7 changed files with 89 additions and 30 deletions
--- 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<SDRMConnector> connector);
--- a/include/aquamarine/backend/drm/Atomic.hpp
+++ b/include/aquamarine/backend/drm/Atomic.hpp
@ -23,10 +23,14 @@ namespace Aquamarine {
        CDRMAtomicRequest(Hyprutils::Memory::CWeakPointer<CDRMBackend> backend);
        ~CDRMAtomicRequest();

+        void setConnector(Hyprutils::Memory::CSharedPointer<SDRMConnector> connector);
        void addConnector(Hyprutils::Memory::CSharedPointer<SDRMConnector> connector, SDRMConnectorCommitData& data);
+        void addConnectorModeset(Hyprutils::Memory::CSharedPointer<SDRMConnector> connector, SDRMConnectorCommitData& data);
+        void addConnectorCursor(Hyprutils::Memory::CSharedPointer<SDRMConnector> connector, SDRMConnectorCommitData& data);
        bool commit(uint32_t flagssss);
        void add(uint32_t id, uint32_t prop, uint64_t val);
        void planeProps(Hyprutils::Memory::CSharedPointer<SDRMPlane> plane, Hyprutils::Memory::CSharedPointer<CDRMFB> fb, uint32_t crtc, Hyprutils::Math::Vector2D pos);
+        void planePropsPos(Hyprutils::Memory::CSharedPointer<SDRMPlane> plane, Hyprutils::Math::Vector2D pos);

        void rollback(SDRMConnectorCommitData& data);
        void apply(SDRMConnectorCommitData& data);
--- 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 {
--- 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<IBuffer> 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<IBuffer> 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);
 }

--- 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<SDRMPlane> 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<SDRMConnector> connector) {
+    conn = connector;
+}
+
 void Aquamarine::CDRMAtomicRequest::addConnector(Hyprutils::Memory::CSharedPointer<SDRMConnector> 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<SDRMConnector> 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<SDRMConnector> 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(
--- 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();

--- 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) {