Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid excessive atomic properties updates #95

Merged
merged 28 commits into from
Jan 10, 2025
Merged

Conversation

UjinT34
Copy link
Contributor

@UjinT34 UjinT34 commented Oct 11, 2024

Splitted addConnector into 3 parts. Modesetting properties and cursor properties go into separate functions. Mixing those in without need might cause increased cpu usage, flickering or break vrr. Most noticeable with intel videocards.
Moved Colorspace and hdr_output_metadata into modeset category.
Removed unnecessary Colorspace and hdr_output_metadata clearing unless explicitly requested.

Submitting any cursor plane property will break vrr on nvidia. This PR skips setting those properties unless something changes. Doesn't fully fix vrr with hw cursors on nvidia but makes it better. Looks like it is a driver bug and nothing else can be done on AQ/HL side. If this will result in brightness fluctuations with vrr and hw cursors enabled than before this PR it was broken and monitor was stuck at max refresh rate. Should be fixed by disabling vrr to have the same result as before the PR or by disabling hw cursors to get fully working vrr without brightness flickers.

@vaxerski
Copy link
Member

results in a new surface commit event

what? We're in DRM here, a "surface" doesn't exist. What are you talking about?

If you don't commit the cursor plane state, I assume it's left unaltered...? Have you checked that?

@UjinT34
Copy link
Contributor Author

UjinT34 commented Oct 11, 2024

"planeProps cursor -> listener_commitWindow -> damageSurface -> CDRMAtomicImpl::commit -> planeProps cursor" goes in a loop. I don't know why it works this way. planeProps(connector->crtc->cursor, ...) make HL think that a new frame should be rendered and rendering a "new" frame results in a new planeProps(connector->crtc->cursor, ...). Something similar happens with sendPointerFrame and locked cursors hyprwm/Hyprland#6877

Looks like it's left unaltered. At least the cursor stays visible and there are no artifacts present.

@vaxerski
Copy link
Member

planeProps cursor -> listener_commitWindow

excuse me?

@UjinT34
Copy link
Contributor Author

UjinT34 commented Oct 11, 2024

You send cursor position and get a _CWlSurfaceCommit which ends up in a listener_commitWindow. No idea why this happens and how to stop it from happening. At least it looks this way. Hard to debug it since something happens outside HL and AQ and triggers this unwanted _CWlSurfaceCommit.

@vaxerski
Copy link
Member

yea like thats the question how the fuck is that related, idk if curing the symptom will help with fixing the underlying issue

gulafaran and others added 22 commits January 7, 2025 23:00
…hyprwm#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.
…yprwm#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: hyprwm#107

Signed-off-by: Yao Zi <[email protected]>
* consolidates into checkOutput for clearer flow when rescanning connectors

* add error log
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
@UjinT34 UjinT34 changed the title Fix VRR with HW cursor enabled Avoid excessive atomic properties updates Jan 9, 2025
@UjinT34
Copy link
Contributor Author

UjinT34 commented Jan 9, 2025

Looks like every or most atomic properties keep their values unless altered so there is no need to repeat them on every commit.
Extra surface commits aren't observed anymore. But mouse movement still breaks vrr on nvidia.
Changes to Colorspace and hdr_output_metadata should fix increased cpu usage on intel (also requires HL update)

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally more or less fine

include/aquamarine/output/Output.hpp Show resolved Hide resolved
src/backend/drm/impl/Atomic.cpp Outdated Show resolved Hide resolved
@vaxerski
Copy link
Member

rest is ok to me

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@vaxerski vaxerski merged commit c2369bc into hyprwm:main Jan 10, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants