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

drm/atomic: Update connector prop mapping in reset_state(), and refactor prop mapping code a bit #1531

Merged
merged 5 commits into from
Sep 19, 2024
Merged

drm/atomic: Update connector prop mapping in reset_state(), and refactor prop mapping code a bit #1531

merged 5 commits into from
Sep 19, 2024

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Sep 9, 2024

Fixes #1374.

@YaLTeR
Copy link
Contributor

YaLTeR commented Sep 9, 2024

If I store some prop handles in my compositor (i.e. GAMMA_LUT) does that mean I need to refresh those at some point too, and when?

@Drakulix
Copy link
Member

Drakulix commented Sep 9, 2024

If I store some prop handles in my compositor (i.e. GAMMA_LUT) does that mean I need to refresh those at some point too, and when?

You should remove and re-evaluate them on hotplug, so if you are using smithay-drm-extras on connector_changed (otherwise in UdevHandler::device_changed, for any new connectors).

Note this only applies to connector properties, crtcs and planes are static anyhow.

@ids1024
Copy link
Member Author

ids1024 commented Sep 9, 2024

If I store some prop handles in my compositor (i.e. GAMMA_LUT) does that mean I need to refresh those at some point too, and when?

This is specifically an issue with connectors, which on some cards can be added and removed. I think once a drm object is created the properties it has are static? And I don't know if planes or CRTCs are ever added dynamically...

@YaLTeR
Copy link
Contributor

YaLTeR commented Sep 9, 2024

Okay I'm not sure I actually store any connector properties, but either way I would store them for each connector as it is connected (and remove that info when it is disconnected). But I do call drm_compositor.reset_state() which is mentioned in the title of this PR, which is why I asked.

@ids1024
drm/atomic: Make Mapping a struct with named fields
710be75 
Even though it's type safe, using tuple indexes to access the fields
seems unnecessarily confusing. It's generally better to have a named
type.
@ids1024
drm/atomic: Make *_prop_handle() methods of PropMapping
f0912b8
@ids1024
drc/atomic: Use *_prop_handle() instead of duplicating same logic
c595325
@ids1024
drm/atomic: Put RwLock<PropMapping> in Arc, and share
24652b2 
If this has to be in an `RwLock` anyway, it seems unnecessary to clone
it, and it can just be shared.

(`AtomicDrmDevice` does not use `RwLock` yet, but will if it wants to
update the mapping.)

I don't expect this to cause problems, though maybe it doesn't help much
either. But it makes sense.
@ids1024
drm/atomic: Update connector prop mapping in reset_state()
3c08aac 
On some cards, connectors may have been added dynamically since the
mapping was first created.

Fixes Smithay#1374.
Drakulix
Drakulix approved these changes Sep 19, 2024
View reviewed changes
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.

LGTM now

@Drakulix Drakulix merged commit fdfdaec into Smithay:master Sep 19, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Drakulix Drakulix Drakulix approved these changes

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

Successfully merging this pull request may close these issues.

prop_mapping cache in AtomicDrmDevice doesn't handle connector hotplug
3 participants
@ids1024 @YaLTeR @Drakulix