How should "max bpc" KMS property work?

[Sebastian Wick]

On Wed, Apr 27, 2022 at 5:41 PM Harry Wentland <harry.wentland at amd.com> wrote:
>
>
>
> On 2022-04-27 06:52, Pekka Paalanen wrote:
> > 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 at linux.intel.com> 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 wanted to remove the 8 bpc limitations for a while now but it
> looks like we never did for anything other than eDP.
>
> The original problem we solved was that some monitors default timing
> couldn't be driven at a high bpc. Therefore users were faced with black
> displays. On some displays you also can't drive high refresh rate modes
> with a higher bpc.
>
> >> 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?
> >
>
> I think both of those options make sense. I'll need to think about the
> automatic fallback if we don't have enough bandwidth for max_bpc.
>
> If a KMS driver falls back automatically we probably want some way
> for a (color managed) compositor to know if the output bpc is reduced.
>
> > Does KMS use dithering automatically, btw?
> >
>
> amdgpu's display driver does.
>
> > 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.
> >
>
> min_bpc sounds like something we might want for HDR use-cases, unless
> the compositor has a way to confirm the output box (and format). min_bpc
> would allow the KMS driver to pick the lowest required bpc so the
> compositor always has a guarantee of quality.

IMO that would be ideal. The driver should try to reduce bandwidth by lowering
the bpc down to the min_bpc if the hardware can't drive the selected mode at a
higher bpc. User space usually knows which bpc is sufficient for the use case
but will never complain about too much bpc. Drivers which don't support
lowering the bpc dynamically can then still go with the min_bpc and user space
still gets all the modes which work with the minimum required bpc.

>
> Harry
>
> >
> > Thanks,
> > pq
>