Skip to content

core: add DisplayInfo::physical_address - #5291

Open
saviq-work wants to merge 2 commits into
fix-display-tics-conversionfrom
MIRENG-2199-edid-physical-address
Open

saviq-work wants to merge 2 commits into
fix-display-tics-conversionfrom
MIRENG-2199-edid-physical-address

Conversation

@saviq-work

Copy link
Copy Markdown
Contributor

What's new?

DisplayInfo::physical_address

How to test

Tests.

Checklist

  • Tests added and pass
  • Adequate documentation added
  • (optional) Added Screenshots or videos

Stack created with GitHub Stacks CLI • Give Feedback 💬

@saviq-work
saviq-work added this pull request to stack #5287 October 1, 2026 17:54
@saviq-work
saviq-work force-pushed the MIRENG-2199-edid-physical-address branch from 005f098 to de17bcc Compare October 1, 2026 21:27
Copilot AI lite review requested due to automatic review settings October 1, 2026 21:27
@saviq-work
saviq-work removed this pull request from stack #5287 October 1, 2026 21:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The libmircore4 Debian symbols manifest is missing after the ABI bump.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds HDMI physical-address parsing to DisplayInfo, with unit tests and Mir Core ABI/package updates.

Changes:

  • Parses CTA HDMI vendor blocks.
  • Adds DisplayInfo::physical_address and test coverage.
  • Bumps the Mir Core ABI and packaging metadata.
File Summary
tests/​unit-tests/​graphics/​test_display_configuration.cpp Adds EDID parsing tests.
src/​core/​graphics/​display_configuration.cpp Parses HDMI physical addresses.
src/​core/​CMakeLists.txt Bumps the ABI; moderate issue (1 vote): add/update debian/libmircore4.symbols.
rpm/​mir.spec Updates RPM SONAME metadata.
include/​core/​mir/​graphics/​display_configuration.h Adds physical_address.
debian/​libmircore4.install Installs the new SONAME.
debian/​libmircore3.install Removes the old SONAME installation.
debian/​control Updates the Debian package; moderate issue (3 votes): add/update the missing symbols manifest.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread debian/control Outdated
@saviq-work
saviq-work force-pushed the MIRENG-2199-edid-physical-address branch from de17bcc to 655e9fc Compare October 1, 2026 21:29
@saviq-work
saviq-work changed the base branch from MIRENG-2045-implement-the-public-output-configuration-policy-api to fix-display-tics-conversion October 1, 2026 21:29
@saviq-work
saviq-work marked this pull request as draft October 1, 2026 21:36
@saviq-work
saviq-work force-pushed the MIRENG-2199-edid-physical-address branch from 655e9fc to 42fcedc Compare October 2, 2026 06:55
@saviq-work
saviq-work added this pull request to stack #5299 October 2, 2026 06:55
@saviq-work
saviq-work marked this pull request as ready for review October 2, 2026 06:56
@saviq-work saviq-work changed the title mircore: add DisplayInfo::physical_address core: add DisplayInfo::physical_address Oct 2, 2026
@saviq-work
saviq-work force-pushed the MIRENG-2199-edid-physical-address branch from 42fcedc to 12bb57b Compare October 2, 2026 07:01

@mattkae mattkae left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool 🫘

@saviq-work
saviq-work removed this pull request from stack #5299 October 3, 2026 17:38
@saviq-work
saviq-work added this pull request to stack #5301 October 3, 2026 17:38
@saviq-work
saviq-work removed this pull request from stack #5301 October 3, 2026 17:39
@saviq-work
saviq-work force-pushed the MIRENG-2199-edid-physical-address branch from 12bb57b to e426372 Compare October 3, 2026 17:43
@saviq-work
saviq-work added this pull request to stack #5303 October 3, 2026 17:55
@saviq-work
saviq-work force-pushed the MIRENG-2199-edid-physical-address branch from e426372 to fdbc88d Compare October 4, 2026 09:42
@saviq-work
saviq-work removed this pull request from stack #5303 October 4, 2026 09:44
@saviq-work
saviq-work added this pull request to stack #5305 October 4, 2026 09:52
@saviq-work
saviq-work force-pushed the MIRENG-2199-edid-physical-address branch 2 times, most recently from c0e8178 to e41d09c Compare October 5, 2026 11:36

@AlanGriffiths AlanGriffiths left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's no need to bump the mircore ABI - DisplayInfo is only in the 2.31 stanza

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

TICS Quality Gate

✔️ Passed

mir

Coding Standards: ✔️ Passed

✔️ Condition “No new Coding Standard Violations for level 1, 2, 3 with respect to Previous analysis” passed.

See the results in the TICS Viewer

The following files have been checked for this project
  • include/core/mir/graphics/display_configuration.h
  • src/core/graphics/display_configuration.cpp

TICS / TICS / Run TICS analysis

@saviq-work

Copy link
Copy Markdown
Contributor Author

There's no need to bump the mircore ABI - DisplayInfo is only in the 2.31 stanza

Ah! Forgot we're only now moving it into core 🍾.

@saviq-work
saviq-work force-pushed the MIRENG-2199-edid-physical-address branch 2 times, most recently from 4a165bd to b14bf2b Compare October 5, 2026 12:45
@saviq-work saviq-work closed this Oct 5, 2026
@saviq-work saviq-work reopened this Oct 5, 2026
Comment thread src/core/graphics/display_configuration.cpp Outdated
@saviq-work
saviq-work force-pushed the MIRENG-2199-edid-physical-address branch from b14bf2b to 70ec433 Compare October 5, 2026 13:56

@AlanGriffiths AlanGriffiths left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[out of scope] but I'm not sure how a uint16_t can represent a physical_address

@saviq-work

Copy link
Copy Markdown
Contributor Author

[out of scope] but I'm not sure how a uint16_t can represent a physical_address

@copilot please explain in a comment to this PR.

Copilot AI commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

[out of scope] but I'm not sure how a uint16_t can represent a physical_address

@copilot please explain in a comment to this PR.

physical_address is the HDMI/EDID topology address, not a byte-addressable memory location. HDMI encodes it in a 16-bit field as four 4-bit path components (commonly displayed in dotted form, e.g. 1.2.0.0), with zero components marking the end of the path. That is why uint16_t is the appropriate representation here.

Co-authored-by: Alan Griffiths <alan@octopull.co.uk>

This branch has not been deployed

No deployments
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.

5 participants