No way to check current active color format

Wouldn't the a DRM property be the canonical route to report this back to userspace (e.g. the goings on in amdgpu_dm_connector_atomic_get_property())?

Thanks for the hint.

Sadly amdgpu_dm_connector_atomic_get_property does not seem to have access to the stream struct currently in use. It only gets "struct drm_connector *connector" and "struct drm_connector_state *state" and I don't see a way to access the dm_crtc_state and therefore the stream object from these structs.

And I also didn't find a get_current_stream function so far.

I wonder if this is really the intended path, because I just want to output values, they can not be set.

DRM properties have their defined generic meanings that are documented as part of the UAPI contract. A property can be write-only, read-write, or read-only, but you have to follow the existing defintion, you can't just pick something and start using it differently than it was intended.

So this depends on whether a suitable DRM property is already defined. Adding support in a driver is "easy". But if you have to define a new DRM property, then the full burden of proving it with real-world userspace patches hits you.

I would assume the same requirement is on new sysfs attributes but on that I am less sure. Whatever it will be, it should be generic: userspace must not need to know which kernel driver is used to be able to use it if it exists.

Are read only properties special in any way or are they implemented by just not providing a setter functionality (like in my poc below)?

Do you have a link where I can read up on the UAPI contract?

I suppose the most important thing to start with are the requirements when introducing any kind of new UAPI: https://www.kernel.org/doc/html/latest/gpu/drm-uapi.html#open-source-userspace-requirements

There is a debate to be had whether Xorg with xrandr alone is enough as a userspace proving vehicle. Generally I'd say no, but for read-only properties it might be enough. But this is just my personal opinion, I'm not a kernel dev.

https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#kms-properties may be a good entrypoint to property docs. There is DRM_MODE_PROP_IMMUTABLE for read-only properties.

Mind, if you are just hooking up an existing DRM property already used by some other driver, then that does not count as new UAPI, unless you use it in new inventive ways that hasn't been done before.

Thanks for the hints

I found some props:

But they are not red only and more like #476 I guess: Setting the desired color format and not the one actually active (which can be different if the desired format is not available for the choosen resolution/refresh rate).

My company, Tuxedo Computers, wants to integrate the information in an open source system settings gui. Would that be enough as a userspace proving vehicle?

My company, Tuxedo Computers, wants to integrate the information in an open source system settings gui. Would that be enough as a userspace proving vehicle?

Sounds good to me, but I'm not the one to decide. :-)

I do want to underline one thing: when you add new UAPI, you need the patches for the userspace proving vehicle reviewed and accepted, but not merged until the kernel patches have been merged properly.

One caveat I seem to remember with DRM/KMS properties -- and I might very well be wrong here -- is the DRM 'master' process gets exclusive access to the device, which restricts access to the properties -- at least for writing. So under Xorg, the server just passes things through on behalf of xrandr (or whatever client). But on Wayland I found I was unable to write properties directly using a simple tool because of -EPERM on /dev/dri/card0; I concluded that managing a property requires the compositor to expose it. I don't know if this applies to reading the properties, though.

You are correct, but I wouldn't call it a caveat. No-one should be able to mess up the display state behind the display server's back.

Reading is fine though, as https://github.com/ascent12/drm_info demonstrates.

added DC label

changed the description

Looking at struct drm_connector_state there's a struct drm_crtc *crtc. Then within struct drm_crtc there's struct drm_crtc_state *state. If this is contained in a struct dm_crtc_state as the base member, the former might be accessible using the container_of macro.

Just thinking out loud here, mind -- there's probably a mistake or a NULL deref lurking in this chain somewhere...

Another thing to try out for me, thanks

PoC patch based on 5.11.0 kernel:

expose_color_info_5.11.0-1.patch

Color mode information can be printed using xrandr --prop

New properties are:

active output color space
active display color depth
active pixel encoding

One question: The strings printed in xrandr are defined with enums in amdgpu_display.c. I wonder: What happens when a new possible value gets added to he kernel that is not in that enum?

Know bugs:

might not show results when display gets connected after boot
might crash when display gets disconnected (dm_crtc_state->stream->* null pointer dereference that needs to be checked?)

mentioned in issue #476

New version of the patch now hopefully without the bugs: https://lkml.org/lkml/2021/5/7/695

I'm currently converting the patch above to a generalized one for amd and intel.

Therefore I'm searching a good place to write the actually choosen color depth and encoding back to drm_connector_state (where I introduced new variables to hold this information).

Candidates are:

dm_update_crtc_state() in amdgpu_dm.c
- is the state always applied? or is this function also used for validation purposes?
amdgpu_dm_atomic_commit_tail() in amdgpu_dm.c
- from teh kernel doc it sounded like this function gets called whenever the crtc settings change, but on my system it gets called repeatedly. Does it have to do with vsync also? aka does it get called every time the framebuffer swaps or something?

Any other idea where I can grab the applied color depth and pixel encdogin every time they get applied/changed?

And another question I could use some help: Is there a way to get all connectors of a crtc or the other way around: get the crtc feeding a connector?

All I found is the connector_mask to check if a given connector is connected to a crtc, but his means that for every crtc I must iterate over all connectors.

The functionality is included in this patchset now: #476 (comment 963469)

@Matombo any luck pushing your kernel patch? Last I saw it stalled with them wanting you to totally redo the exposed config settings to all match, which I think is a bit unfair.

No, the current situaion I got was that no good idea is on the table for the current api and an api rework will come when wayland reaches the point where color management will be implemented.

So, that could take a while.

Note that the patch still isn't bugfree, but for the moment I stalled my work. Hope that wayland moves fast ^^.

This issue hasn't had any activity since 2022-03-01. The AMD driver stack changes rapidly and contains lots of shared code across products so it's possible that it has already been fixed. Please upgrade to a current stable kernel and userspace stack and try again. If you still experience this issue with the latest driver stack, please capture relevant logging and open a new issue referring back to this one.

I don't feel it's correct to close this issue when the issue still exists.
#476 is a work in progress and hasn't been merged yet.

How about we mark it as a duplicate? No need to have two issues open for the same thing.

closed

added feature-request label

marked this issue as a duplicate of #476

marked this issue as related to #476

No way to check current active color format

Designs

Child items ...

Activity

Admin message

Admin message

No way to check current active color format

Activity