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.

Sign up for GitHub

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

Jump to bottom

Initial Atomic KMS platform #3525

Open
wants to merge 4 commits into
base: MIRENG-653/platform-specific-options
Choose a base branch
from
Open

Initial Atomic KMS platform #3525

wants to merge 4 commits into from

Conversation

RAOF
Copy link
Contributor

@RAOF RAOF commented Aug 5, 2024

This is a minimal-changes copy of the display half of the gbm-kms platform, switched over to all Atomic KMS API.

It's parallel installable with the existing gbm-kms platform, and will be preferentially used on devices with atomic support.

Currently, over the existing gbm-kms display platform it only has improved gamma support.
Immediate missing features:

  • Hardware cursor support
  • Querying list of supported pixel formats

@RAOF RAOF requested a review from a team as a code owner August 5, 2024 07:16
AlanGriffiths
AlanGriffiths reviewed Aug 5, 2024
View reviewed changes
src/platforms/atomic-kms/server/kms/quirks.cpp
Comment on lines +182 to +185
config.add_options()
(quirks_option_name,
boost::program_options::value<std::vector<std::string>>(),
"[platform-specific] Driver quirks to apply (may be specified multiple times; multiple quirks are combined)");
Copy link
Contributor

Choose a reason for hiding this comment

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

This same option is added by gbm-kms - and that spells trouble if both platforms are available. (Similar to #885)

@RAOF RAOF force-pushed the initial-atomic-kms-platform branch from 34f45ee to 8bf1aca Compare August 8, 2024 03:37
@RAOF RAOF force-pushed the initial-atomic-kms-platform branch from 8bf1aca to 280f6ed Compare September 9, 2024 02:05
@RAOF RAOF changed the base branch from main to MIRENG-653/platform-specific-options September 9, 2024 02:06
@RAOF RAOF force-pushed the MIRENG-653/platform-specific-options branch from 415a9c3 to 9367dc7 Compare September 9, 2024 05:42
@RAOF RAOF force-pushed the initial-atomic-kms-platform branch 2 times, most recently from a20a6cf to ae1a6ac Compare September 10, 2024 04:45
@RAOF RAOF force-pushed the MIRENG-653/platform-specific-options branch from 5297983 to 7fd62d0 Compare September 10, 2024 08:04
@RAOF RAOF force-pushed the initial-atomic-kms-platform branch from ae1a6ac to d172bfa Compare September 10, 2024 08:10
@RAOF
platforms/common: Add mgk::get_cap_checked()
abf313c
@RAOF
platforms/common: Add missing mgk::ObjectProperties implementations
fd2ca5a
@RAOF
platforms: Add atomic-kms platform
6e64d62 
This is initially a copy of the display half of `gbm-kms`, quickly ported to use
only the atomic KMS APIs.

It shall be further developed to usefully use the atomic APIs to fix various TODOs,
and provide support for extra performance features
@RAOF RAOF force-pushed the initial-atomic-kms-platform branch from d172bfa to 6e64d62 Compare September 12, 2024 07:48
@Saviq
Copy link
Collaborator

Saviq commented Sep 24, 2024

Hmm with dual outputs I get:

< - ERROR - > atomic-kms: Failed to set CRTC: No space left on device (28)

And an occasional SIGABRT after.

Yes. I. Have. Disk. Space.

@Saviq
Copy link
Collaborator

Saviq commented Sep 24, 2024 •
edited
Loading

@tarek-y-ismail's ploughed through drm debug logs and found:

[drm:drm_atomic_plane_check] [PLANE:32:plane 1A] invalid source coordinates 2256.000000x1504.000000+0.000000+0.000000 (fb 1920x1200)
[drm:drm_atomic_check_only] [PLANE:32:plane 1A] atomic core check failed

This suggests there's a problem with cloning, as it tries to map the smaller buffer onto a higher resolution display.

@tarek-y-ismail
Copy link
Contributor

tarek-y-ismail commented Sep 30, 2024 •
edited
Loading

Hmm with dual outputs I get:

By "dual outputs", do you mean builtin-monitor + external monitor, or two external monitors?
I don't get this error, but I also don't get external output (no parameters passed except --platform-display-libs=mir:atomic-kms --platform-rendering-libs=mir:gbm-kms)

@Saviq
Copy link
Collaborator

Saviq commented Sep 30, 2024

By "dual outputs", do you mean builtin-monitor + external monitor, or two external monitors?

Bulting + external.

I don't get this error, but I also don't get external output.

As discussed, to get output through your Nvidia GPU, you'll need ,mir:eglstream-kms added to your display plugins.

@tarek-y-ismail
Copy link
Contributor

Just tried it, both monitors work fine. I assume this is because the second output is not using atomic-kms and thus not triggering the bug

@Saviq
Copy link
Collaborator

Saviq commented Sep 30, 2024

Just tried it, both monitors work fine. I assume this is because the second output is not using atomic-kms and thus not triggering the bug

Yup, that could well be it - they're not sharing the platform like in my case.

@RAOF @tarek-y-ismail @AlanGriffiths
platforms/atomic-kms: Populate supported pixel formats correctly (#3596)
608dfb2 
`DisplayConfigurationOutput.pixel_formats` now contains the list of
accepted pixel formats (or, at least, those pixel formats that are
representable in the `MirPixelFormats` enum; many aren't).

---------

Co-authored-by: tarek-y-ismail <[email protected]>
Co-authored-by: Alan Griffiths <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlanGriffiths AlanGriffiths AlanGriffiths left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RAOF @Saviq @tarek-y-ismail @AlanGriffiths