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

Introduce DRM abstractions crate #924

Merged
merged 3 commits into from
Mar 30, 2023

Conversation

PolyMeilex
Copy link
Member

@PolyMeilex PolyMeilex commented Mar 6, 2023

Smithay DRM Extras

This crate contains some extra abstractions and helpers over DRM

  • edid module is responsible for extraction of information from DRM connectors (for now model and manufacturer)
  • drm_scanner module contains helpers for detecting connector connected and disconnected events as well as mapping crtc to them.
    • ConnectorScanner is responsible for tracking connected/disconnected events.
    • CrtcMapper trait and SimpleCrtcMapper are meant for mapping crtc to connector.
    • DrmScanner<CrtcMapper> combines two above into single abstraction. If it does not fit your needs you can always drop down to using ConnectoScanner alone.

For envisioned usage, see smithay-drm-extras/examples/simple.rs

Anvil Changes

  • Removal of scan_connectors function, surfaces are created on connector connected/plugged event
  • Proper handling of device changed event, backends are no longer scrapped on every hotplug, as connected/disconnected events do this already.
  • General removal of duplicated logic between init/device added/device changed functions, everything is now handled on connected event
Draft
  • Because it has no docs whatsoever
  • I'm happy with connector scanner, but I'm still not sure if I like crtc mapper trait, or in general how it is used in DrmScanner (it should be possible to do multiple connectors per drm surface using it, but it might become annoying, not sure how common it is, is that meant for output mirroring?"
  • Depends on gbm.rs git version

@Drakulix
Copy link
Member

Drakulix commented Mar 7, 2023

First impression from me are, that this is a super nice cleanup. This also introduces a good place to add further connector metadata processing the future. (Looking a libdisplay-info and HDR metadata specifically.)

I'm happy with connector scanner, but I'm still not sure if I like crtc mapper trait, or in general how it is used in DrmScanner

The trait looks very versatile, I don't necessarily see anything wrong with it

(it should be possible to do multiple connectors per drm surface using it, but it might become annoying, not sure how common it is, is that meant for output mirroring?)

Yes, that is hardware output mirroring.

Doing it in software is also possible either by

  • scanning out the same buffer on both connectors, which I don't think either DrmCompositor or GbmBufferedSurface can really do, given they deal with just one DrmSurface.
  • copying the buffer (which becomes a necessity, if the connectors are connected to different gpus.)

So while I like the current approach, we probably want a more complete CrtcMapper in the future, that allows at runtime to enable output-mirroring configurations.
Though something like that can easily be prototyped downstream and then upstreamed into smithay-drm-extras later.

@i509VCB Was thinking about doing an extra crate for DrmNode and the related functions. Maybe we want to start the journey of splitting up smithay and just name this smithay-drm? And possibly move even more of backend::drm into it?
Something more high-level on top of drm-rs (which has a well defined by rather limited scope) is certainly a good idea for the whole ecosystem. (Maybe even interesting for efforts like rust-windowing/winit#2272 ?)

@PolyMeilex PolyMeilex force-pushed the drm-abstractions branch 3 times, most recently from 34c58bc to 3016bb8 Compare March 24, 2023 15:38
@PolyMeilex
Copy link
Member Author

PolyMeilex commented Mar 24, 2023

This is now ready, except for gbm.rs release

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

Approved on the premise of switching to stable gbm.rs.
0.12 release is currently in the CI pipeline: https://github.com/Smithay/gbm.rs/actions/runs/4563939258

@PolyMeilex PolyMeilex marked this pull request as ready for review March 30, 2023 13:38
@PolyMeilex PolyMeilex merged commit 4dd0111 into Smithay:master Mar 30, 2023
@PolyMeilex PolyMeilex deleted the drm-abstractions branch March 30, 2023 13:50
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.

2 participants