Hi Ville and Alex, thanks for the replies. More below. TL;DR: My take-away from this is that I should slam 'max bpc' to the max by default, and offer a knob for the user in case they want to lower it. On Tue, 26 Apr 2022 20:55:14 +0300 Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Tue, Apr 26, 2022 at 11:35:02AM +0300, Pekka Paalanen wrote: > > Hi all, > > > > I'm working on setting HDR & WCG video modes in Weston, and I thought > > setting "max bpc" KMS property on the connector would be a good idea. > > I'm confused about how it works though. > > > > I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612 > > > > Summary: > > > > - Apparently the property was originally added as a manual workaround > > for sink hardware behaving badly with high depth. A simple end user > > setting for "max bpc" would suffice for this use. > > > > - Drivers will sometimes automatically choose a lower bpc than the "max > > bpc" value, but never bigger. > > > > - amdgpu seems to (did?) default "max bpc" to 8, meaning that I > > definitely want to raise it. > > I've occasionally pondered about doing the same for i915, just to have > the safest default possible. But I'd hate to lose the deep color testing > coverage knowing very few people would in practice raise the limit. > Also the number of systems where deep color doesn't work reliably > (or can't be made to work by not using a crap cable) seems to be quite > low. I think when HDR and WCG get into display servers, setting 'max bpc' will become a standard action. It's bit moot to e.g. render everything in electrical 10 bit RGB, if the link is just going to squash that into electrical 8 bit RGB, right? So even 10 bit color would require setting 'max bpc' to at least 10 to be able to actually see it, source-side dithering aside. > > > > If I always slam "max bpc" to the highest supported value for that > > property, do I lose more than workarounds for bad sink hardware? > > We don't have any workarounds implemented like this in the kernel. > Or should not have at least. "max bpc" exists purely for the user > to have a say in the matter in addition to whatever the EDID/quirks > say. Ie. if the kernel knows for sure that deep color won't work on > a particular setup then it should just not allow deep color at all > despite what the prop value says. > > So the only danger is fighting with the user's wishes which I guess > you can overcome with some kind of user visible knob. Right, good. Furthermore, as a KMS client cannot make much assumptions about the KMS state it inherits from some other KMS client, it should know and program all possible KMS properties according to its own desires anyway. That, and the DRM master concept make sure that there cannot be any "third party" KMS configuration programs, like V4L2 has. > > Do I lose the ability to set video modes that take too much bandwidth > > at uncapped driver-selected bpc while capping the bpc lower would allow > > me to use those video modes? > > > > Or, are drivers required to choose a lower-than-usual but highest > > usable bpc to make the requested video mode squeeze through the > > connector and link? > > IMO drivers should implement the "reduce bpc until it fits" > fallback. We have that in i915, except for MST where we'd need > to potentially involve multiple streams in the fallback. That > is something we intend to remedy eventually but it's not an > entirely trivial thing to implement so will take some actual > work. ATM we just cap MST to <=8bpc to avoid users getting into > this situation so often. Excellent, but judging from what Alex said, this is also not what amdgpu does. We have two drivers doing different things then? So with Weston I probably have to document, that if you can't get the video mode you want working, try turning the 'max bpc' knob down and try again. Or, I could cap 'max bpc' based on my framebuffer depth. If I have an electrical 8 bit FB (default in Weston), then there is not much use for having 'max bpc' > 8. This ignores the KMS color pipeline a bit. Does that make sense? Does KMS use dithering automatically, btw? The only mention of dithering in KMS docs is some nouveau-specific KMS properties and another for radeon. > > > > Do I need to implement a fallback strategy in a display server, > > starting from the highest possible "max bpc" value, and if my modeset > > is rejected, repeatedly try with lower "max bpc" setting until it works > > or I'm out of bpc options? > > IMO the bpc part should be handled by the kernel since we already > had this behaviour even before the "max bpc" prop was introduced > and we didn't add an explicit "use this bpc or fail" prop. But of > course you should have some kind of sensible fallback strategy for > things that just fail for other reasons. Right, but this means that I don't have to add 'max bpc' as yet another dimension in the combinatorial explosion of KMS parameters I would need to search to find a working setup. That's really good. The one thing missing is seeing what format and bpc we actually got on the link. > The one problem we have in the kernel is that we have no way to > ask the user if the display we tried to light up is actually > working. So our policy decisions can't really involve user input. > Userspace should not generally have that problem. Indeed. Also like Alex said, the kernel does not know if the user prefers high color depth or high refresh rate either. One way to solve that is to light up the requested video mode any way the kernel can, and then report what the resulting color depth is. Another way is to have explicit "use this bpc or fail" KMS property, maybe in the form of 'min bpc' as I recall being discussed in the past, and let userspace guess what might work. The former is easier to light up, but probing requires actually setting the modes. The latter may require a lot of trial-and-error from userspace to find anything that works, but it takes only time and not blinking - as far as things can be detected with TEST_ONLY commits. Then one still has to ask the user if it actually worked. Thanks, pq
Attachment:
pgpu1LiXRCXwR.pgp
Description: OpenPGP digital signature
