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

scan only cards and not outputs, safeguard against null renderer #106

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

gulafaran
Copy link
Contributor

seems pointless scanning the card[0-9]-eDP etc outputs for kms/bootvga/rendernodes just use the [0-9] cards, also safegaurd in commitstate for missing renderer prevents a coredump if __EGL_VENDOR_LIBRARY_FILENAMES is set to mesa on a laptop with dual gpus and a monitor attached to the nvidia dgpu. it might be a way to make this handled better but now it doesnt segfault atleast with a null dereference to renderer.

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.
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.
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.

lgtm

@vaxerski vaxerski merged commit 3b00e96 into hyprwm:main Nov 13, 2024
1 check passed
@JManch
Copy link
Contributor

JManch commented Nov 28, 2024

After this commit I can no longer start Hyprland in a virtual machine. In case this helps:

$ ls -l /sys/class/drm
lrwxrwxrwx    - root 28 Nov 17:05 card0 -> ../../devices/pci0000:00/0000:00:08.0/drm/card0
lrwxrwxrwx    - root 28 Nov 17:05 card0-Virtual-1 -> ../../devices/pci0000:00/0000:00:08.0/drm/card0/card0-Virtual-1
lrwxrwxrwx    - root 28 Nov 17:05 renderD128 -> ../../devices/pci0000:00/0000:00:08.0/drm/renderD128
.r--r--r-- 4.1k root 28 Nov 17:05 version

@gulafaran
Copy link
Contributor Author

gulafaran commented Nov 28, 2024

After this commit I can no longer start Hyprland in a virtual machine. In case this helps:

$ ls -l /sys/class/drm
lrwxrwxrwx    - root 28 Nov 17:05 card0 -> ../../devices/pci0000:00/0000:00:08.0/drm/card0
lrwxrwxrwx    - root 28 Nov 17:05 card0-Virtual-1 -> ../../devices/pci0000:00/0000:00:08.0/drm/card0/card0-Virtual-1
lrwxrwxrwx    - root 28 Nov 17:05 renderD128 -> ../../devices/pci0000:00/0000:00:08.0/drm/renderD128
.r--r--r-- 4.1k root 28 Nov 17:05 version

Doubt it, both are links to same device, card0 exist. logs?, nvm didnt scroll right enough yeah indeed, 00:08.0/drm/card0/card0-Virtual-1

@gulafaran
Copy link
Contributor Author

After this commit I can no longer start Hyprland in a virtual machine. In case this helps:

$ ls -l /sys/class/drm
lrwxrwxrwx    - root 28 Nov 17:05 card0 -> ../../devices/pci0000:00/0000:00:08.0/drm/card0
lrwxrwxrwx    - root 28 Nov 17:05 card0-Virtual-1 -> ../../devices/pci0000:00/0000:00:08.0/drm/card0/card0-Virtual-1
lrwxrwxrwx    - root 28 Nov 17:05 renderD128 -> ../../devices/pci0000:00/0000:00:08.0/drm/renderD128
.r--r--r-- 4.1k root 28 Nov 17:05 version

Doubt it, both are links to same device, card0 exist. logs?, nvm didnt scroll right enough yeah indeed, 00:08.0/drm/card0/card0-Virtual-1

Technically virtual1 should be your "output" yeah, logs?

@gulafaran
Copy link
Contributor Author

Broken launch
Successful launch after reverting this commit

ah so i was focusing on the wrong commit out of the two that merged, you are hitting the other one.

[ERR] [AQ] drm: No renderer attached to backend curious how its even working at all, should null ptr deref and crash then.

@JManch
Copy link
Contributor

JManch commented Nov 28, 2024

Sorry those initial logs I shared were bad. Here they are with proper traces:
bad.log
good.log
Good is with this commit reverted.

UjinT34 pushed a commit to UjinT34/aquamarine that referenced this pull request Jan 9, 2025
…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.
vaxerski added a commit that referenced this pull request Jan 10, 2025
* 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: #107

Signed-off-by: Yao Zi <[email protected]>

* 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 a132fa4 (#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 <[email protected]>
Co-authored-by: Tom Englund <[email protected]>
Co-authored-by: Vaxry <[email protected]>
Co-authored-by: Ziyao <[email protected]>
Co-authored-by: Ikalco <[email protected]>
Co-authored-by: Mihai Fufezan <[email protected]>
Co-authored-by: Austin Horstman <[email protected]>
Co-authored-by: Richard Henninger <[email protected]>
Co-authored-by: Jan Beich <[email protected]>
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.

3 participants