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

Use old DRM cursor API as a placeholder for the atomic KMS hardware cursor #3665

Merged
merged 5 commits into from
Nov 18, 2024
Merged

Use old DRM cursor API as a placeholder for the atomic KMS hardware cursor #3665

merged 5 commits into from
Nov 18, 2024

Conversation

tarek-y-ismail
Copy link
Contributor

This code will act as a placeholder temporarily until we implement damage, at which point, the real atomic KMS cursor code (#3615) can be updated and merged.

@tarek-y-ismail tarek-y-ismail self-assigned this Nov 8, 2024
RAOF
RAOF reviewed Nov 11, 2024
View reviewed changes
Copy link
Contributor

@RAOF RAOF left a comment

Choose a reason for hiding this comment

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

Yup, that looks about right.

You can remove all the cursor plane stuff, and it needs conflicts resolving (there's locking around the current configuration, now).

src/platforms/atomic-kms/server/kms/cursor.cpp Outdated Show resolved Hide resolved
src/platforms/atomic-kms/server/kms/cursor.cpp Outdated Show resolved Hide resolved
src/platforms/common/server/kms-utils/kms_connector.cpp Outdated Show resolved Hide resolved
@tarek-y-ismail tarek-y-ismail force-pushed the atomic-kms-hardware-cursor-but-not-really-atomic branch from 8354a3c to a1a0bcb Compare November 11, 2024 07:56
@tarek-y-ismail tarek-y-ismail force-pushed the atomic-kms-hardware-cursor-but-not-really-atomic branch from a1a0bcb to 5d9d914 Compare November 11, 2024 08:04
RAOF
RAOF requested changes Nov 14, 2024
View reviewed changes
Copy link
Contributor

@RAOF RAOF left a comment

Choose a reason for hiding this comment

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

Apart from the mis-locking, this looks good. (Notably, in case anyone else wants to review, the major diff in cursor.cpp and cursor.h are a straight copy + namespace change of the same files from gbm-kms).

src/platforms/atomic-kms/server/kms/atomic_kms_output.cpp Outdated Show resolved Hide resolved
src/platforms/atomic-kms/server/kms/atomic_kms_output.cpp Outdated Show resolved Hide resolved
src/platforms/atomic-kms/server/kms/atomic_kms_output.cpp Outdated Show resolved Hide resolved
@tarek-y-ismail tarek-y-ismail marked this pull request as ready for review November 14, 2024 08:49
@tarek-y-ismail tarek-y-ismail requested a review from a team as a code owner November 14, 2024 08:49
AlanGriffiths
AlanGriffiths approved these changes Nov 14, 2024
View reviewed changes
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Just nitpicking

src/platforms/atomic-kms/server/kms/atomic_kms_output.cpp Outdated Show resolved Hide resolved
src/platforms/atomic-kms/server/kms/atomic_kms_output.cpp Outdated Show resolved Hide resolved
@tarek-y-ismail
Compact more locking logic in AtomicKMSOuput
947f7c3
@tarek-y-ismail
Change AtomicKMSOutput::set_cursor to not return anything.
5f598f3 
After a discussion with Chris, we concluded that `set_cursor`'s return
value does not matter (i.e. we can't do much about an error except
logging it).
RAOF
RAOF approved these changes Nov 18, 2024
View reviewed changes
Copy link
Contributor

@RAOF RAOF left a comment

Choose a reason for hiding this comment

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

Yup, that's resolved the comments.

@tarek-y-ismail tarek-y-ismail added this pull request to the merge queue Nov 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 18, 2024
@tarek-y-ismail tarek-y-ismail added this pull request to the merge queue Nov 18, 2024
@tarek-y-ismail tarek-y-ismail mentioned this pull request Nov 18, 2024
Merged
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 18, 2024
@tarek-y-ismail tarek-y-ismail added this pull request to the merge queue Nov 18, 2024
Merged via the queue into main with commit 1771170 Nov 18, 2024
36 of 39 checks passed
@tarek-y-ismail tarek-y-ismail deleted the atomic-kms-hardware-cursor-but-not-really-atomic branch November 18, 2024 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlanGriffiths AlanGriffiths AlanGriffiths approved these changes

@RAOF RAOF RAOF approved these changes

Assignees

@tarek-y-ismail tarek-y-ismail

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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