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

Add dumb allocator #31

Closed
wants to merge 4 commits into from
Closed

Add dumb allocator #31

wants to merge 4 commits into from

Conversation

UjinT34
Copy link
Contributor

@UjinT34 UjinT34 commented Jul 25, 2024

  • Adds dumb allocator and buffer implementation. Mostly copied from gbm and kwin.
  • Adds fallbackAllocator API

fallbackAllocator defaults to nullptr, returns dumb allocator for drm backend.

hyprwm/Hyprland#7040

include/aquamarine/allocator/GBM.hpp Outdated Show resolved Hide resolved
include/aquamarine/allocator/GBM.hpp Outdated Show resolved Hide resolved
include/aquamarine/allocator/GBM.hpp Outdated Show resolved Hide resolved
include/aquamarine/allocator/GBM.hpp Outdated Show resolved Hide resolved
include/aquamarine/buffer/Buffer.hpp Outdated Show resolved Hide resolved
@moetayuko
Copy link
Contributor

I tested this PR and hyprwm/Hyprland#7040, they broke my hw cursor
hyprland.log

[hc] loadThemeStyle: loading for size 48
[hc] getShapesC: found 1 images for left_ptr
[TRACE] [AQ] atomic moveCursor
[TRACE] [AQ] CDRMOutput::scheduleFrame: reason 4, needsFrame true, isPageFlipPending true, frameEventScheduled true
[TRACE] [AQ] GBM: Allocating a buffer: size [Vector2D: x: 128, y: 128], format INVALID, cursor: true, multigpu: true, scanout: true
[LOG] [AQ] GBM: Automatically selected format AR24 for new GBM buffer
[LOG] [AQ] GBM: Buffer is marked as multigpu, forcing linear
[TRACE] [AQ] GBM: Using modifier-based allocation, modifiers: 1
[TRACE] [AQ] GBM: | mod 0x0
[ERR] [AQ] GBM: Allocating with modifiers and flags failed for cursor plane, falling back to dumb
[ERR] [AQ] Couldn't allocate a gbm buffer with size [Vector2D: x: 128, y: 128] and format INVALID
[TRACE] [AQ] GBM: Allocating a dumb buffer: size [Vector2D: x: 128, y: 128], format INVALID
[LOG] [AQ] GBM: Allocated a new dumb buffer with size [Vector2D: x: 128, y: 128] and format AR24
[TRACE] [AQ] GBM: Allocating a buffer: size [Vector2D: x: 128, y: 128], format INVALID, cursor: true, multigpu: true, scanout: true
[LOG] [AQ] GBM: Automatically selected format AR24 for new GBM buffer
[LOG] [AQ] GBM: Buffer is marked as multigpu, forcing linear
[TRACE] [AQ] GBM: Using modifier-based allocation, modifiers: 1
[TRACE] [AQ] GBM: | mod 0x0
[ERR] [AQ] GBM: Allocating with modifiers and flags failed for cursor plane, falling back to dumb
[ERR] [AQ] Couldn't allocate a gbm buffer with size [Vector2D: x: 128, y: 128] and format INVALID
[TRACE] [AQ] GBM: Allocating a dumb buffer: size [Vector2D: x: 128, y: 128], format INVALID
[LOG] [AQ] GBM: Allocated a new dumb buffer with size [Vector2D: x: 128, y: 128] and format AR24
[LOG] [AQ] Swapchain: Reconfigured a swapchain to [Vector2D: x: 128, y: 128] AR24 of length 2
[TRACE] Failed to create cursor RB with format 875713089, mod 0
[TRACE] [pointer] hw cursor failed rendering
[TRACE] [pointer] hw transformed hotspot for DP-1: [Vector2D: x: 10, y: 3]
[TRACE] [AQ] CDRMOutput::scheduleFrame: reason 2, needsFrame true, isPageFlipPending true, frameEventScheduled true
[TRACE] [AQ] CDRMOutput::scheduleFrame: reason 3, needsFrame true, isPageFlipPending true, frameEventScheduled true
[TRACE] [AQ] CDRMOutput::scheduleFrame: reason 3, needsFrame true, isPageFlipPending true, frameEventScheduled true
[TRACE] Output DP-1 rejected hardware cursors, falling back to sw

my cursor buffer is expected to be allocated by the modifier-less allocation path:

if (!bo) {
if (explicitModifiers.size() == 1 && explicitModifiers[0] == DRM_FORMAT_MOD_LINEAR) {
flags |= GBM_BO_USE_LINEAR;
allocator->backend->log(AQ_LOG_ERROR, "GBM: Allocating with modifiers failed, falling back to modifier-less allocation");
} else
allocator->backend->log(AQ_LOG_ERROR, "GBM: Allocating with modifiers failed, falling back to implicit");
bo = gbm_bo_create(allocator->gbmDevice, attrs.size.x, attrs.size.y, attrs.format, flags);
}
it's short-circuited by the dumb buffer path w/ this PR

@UjinT34
Copy link
Contributor Author

UjinT34 commented Jul 26, 2024

You need either cursor:allow_dumb_copy = true or cursor:allow_slow_copy for this to work.
Logs [pointer] performing slow copy or [pointer] performing dumb copy.
If you have working hw cursors on main without cursor:allow_dumb_copy = true then you don't need this PR. And after required fixes gbm allocator will work as before.

@moetayuko
Copy link
Contributor

You need either cursor:allow_dumb_copy = true or cursor:allow_slow_copy for this to work. Logs [pointer] performing slow copy or [pointer] performing dumb copy. If you have working hw cursors on main without cursor:allow_dumb_copy = true then you don't need this PR. And after required fixes gbm allocator will work as before.

I have working hw cursor on main. To avoid regression I guess the dumb buffer allocation should be moved after gbm_bo_create to avoid short-circuiting it?

@UjinT34 UjinT34 changed the title Allocate dumb buffer if gbm fails Add dumb allocator Jul 26, 2024
@UjinT34
Copy link
Contributor Author

UjinT34 commented Jul 26, 2024

No regression here. GBM allocator logic restored.
There is no point to move dumb allocator after implicit allocation. Implicit works on nvidia but it'll return an unsuitable buffer for cursor plane. That's why it allows non-renderable linear buffer just before the implicit. Might not be needed with GBM_BO_USE_LINEAR flag though. Didn't test it.

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.

sorry for the delay, on a "break"

src/allocator/Dumb.cpp Outdated Show resolved Hide resolved
src/allocator/Dumb.cpp Outdated Show resolved Hide resolved
src/allocator/Dumb.cpp Outdated Show resolved Hide resolved
@@ -77,7 +77,10 @@ namespace Aquamarine {
virtual std::vector<SDRMFormat> getCursorFormats() = 0;
virtual bool createOutput(const std::string& name = "") = 0; // "" means auto
virtual Hyprutils::Memory::CSharedPointer<IAllocator> preferredAllocator() = 0;
virtual std::vector<SDRMFormat> getRenderableFormats(); // empty = use getRenderFormats
virtual Hyprutils::Memory::CSharedPointer<IAllocator> fallbackAllocator() {
Copy link
Member

Choose a reason for hiding this comment

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

no, no, no. The dumb allocator doesn't really hold any big allocs, so it should be per-drm-backend (can be always present, no big deal)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't really understand what needs to be done here. How HL should get access to dumb allocator?

Copy link
Member

@vaxerski vaxerski Jul 26, 2024

Choose a reason for hiding this comment

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

in CDRMBackend SP<CDumbAllocator>

Copy link
Member

Choose a reason for hiding this comment

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

then it can ->output->backend, cast that to drm and ->dumballocator

include/aquamarine/allocator/Dumb.hpp Outdated Show resolved Hide resolved
@vaxerski
Copy link
Member

vaxerski commented Nov 9, 2024

@UjinT34 can you take a look at #104 ?

That is my implementation, which supersedes this one. However, idk how much you tested it, but for me on AMD it makes my entire driver die, so if you could take a look it'd be nice.

Anyways, closing this one in favor of 104.

@vaxerski vaxerski closed this Nov 9, 2024
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