On Thu, Jun 02, 2022 at 10:47:59AM +0300, Pekka Paalanen wrote: > On Wed, 1 Jun 2022 17:06:25 +0300 > Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > > On Wed, Jun 01, 2022 at 10:21:26AM +0300, Pekka Paalanen wrote: > > > On Tue, 31 May 2022 20:37:31 +0300 > > > Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > > > > > > On Wed, May 25, 2022 at 01:36:47PM +0300, Pekka Paalanen wrote: > > > > > On Wed, 25 May 2022 09:23:51 +0000 > > > > > Simon Ser <contact@xxxxxxxxxxx> wrote: > > > > > > > > > > > On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@xxxxxxxxxxx> wrote: > > > > > > > > > > > > > > Mind that "max bpc" is always in auto. It's only an upper limit, with > > > > > > > > the assumption that the driver can choose less. > > > > > > > > > > > > > > It seems to me like the "max bpc" property is just kind of bad API, > > > > > > > and trying to tweak it to cater to more use cases as we discover them > > > > > > > will take us down a rabbit hole. It seems better to replace it with > > > > > > > new properties which allow user space to determine the current > > > > > > > effective bpc and to explicitly control it. > > > > > > > > > > > > +1, this sounds much more robust, and allows better control by user-space. > > > > > > User-space needs to have fallback logic for other state as well anyways. > > > > > > If the combinatorial explosion is too much, we should think about optimizing > > > > > > the KMS implementation, or improve the uAPI. > > > > > > > > > > +1 as well, with some recommendations added and the running off to the > > > > > sunset: > > > > > > > > > > Use two separate KMS properties for reporting current vs. > > > > > programming desired. The KMS property reporting the current value > > > > > must be read-only (immutable). This preserves the difference between > > > > > what userspace wanted and what it got, making it possible to read > > > > > back both without confusing them. It also allows preserving driver behaviour > > > > > > > > I don't see much real point in a property to report the current bpc. > > > > That can't be used to do anything atomically. So I suppose plymouth > > > > would be the only user. > > > > > > Hi Ville, > > > > > > I think also professional color managed display servers would need it. > > > > > > If they detect that the link bpc is no longer the same as it was when > > > the monitor was profiled, the profile will need to be re-verified by > > > measuring the monitor again. > > > > > > See "Color calibration auditing system" notes in > > > https://gitlab.freedesktop.org/wayland/weston/-/issues/467 description. > > > > > > > So IMO if someone really need explicit control over this then we > > > > should just expose properties to set things explicitly. So we'd > > > > need at least props for the actual bpc and some kind of color > > > > encoding property (RGB/YCbCr 4:4:4/4:2:2:/4:2:0, etc.). And someone > > > > would really need to figure out how DSC would interact with those. > > > > > > I believe there still must be "auto" setting for bpc, and a separate > > > feedback property, so that userspace can use "auto" to see what it can > > > get without doing thousands of TEST_ONLY commits plus a few "link > > > status" failure handlings in order to find a working configuration (I'm > > > assuming we have many more properties than just "max bpc" to figure > > > out). After "auto" has told userspace what actually works without blind > > > trial and error, then userspace can program than value explicitly to > > > make sure it doesn't change accidentally in the future. > > > > Yeah we need "auto", but IMO mainly just to keep the current userspace > > working. Using that to probe what's possible doesn't sound particularly > > workable since you can't use it with TEST_ONLY commits. Also change to > > some other property could still cause the whole thing to fail after the > > max bpc has been probed so not sure it really buys you anything. > > Hi Ville, > > earlier in this thread I drafted how the property-pair with "auto" > could be made useful also with TEST_ONLY commits: > > > Thinking even further, about the problem of TEST_ONLY commits not > > telling you what "auto" settings would actually give you; there could > > be a new/extended KMS ioctl that would be the same as commit now, but > > allow the kernel to return another set of KMS state back with > > TEST_ONLY. Like reading back all KMS state after commit was done for > > real. The "current" KMS properties would be part of that set, and tell > > userspace what would happen in a real commit. > > I do believe the combinatorial explosion of the KMS state search space > to find a working configuration is going to be a very real problem > sooner or later. That's seems like an orthogonal issue that would need some kind of new uapi approach that let's you get some state back out from TEST_ONLY commits. > > > Now that you mentioned some kind of color encoding property (I assume > > > you referred mostly to the sub-sampling aspect), how does the connector > > > property "Colorspace" factor in? > > > > The "Colorspace" property just changes what we report to the display > > via infoframes/MSA/SDP. It does not cause the display hardware to do > > any colorspace conversion/etc. > > Good. > > > To actually force the display hardware to do the desired conversion > > we need some new properties. ATM the only automagic conversion that > > can happen (at least with i915) is the RGB 4:4:4->YCbCr 4:2:0 fallback, > > which is needed on some displays to get 4k+ modes to work at all. > > When "Colorspace" says RGB, and the automatic fallback would kick in to > create a conflict, what happens? I would put that in the "Doctor, it hurts when I..." category. > > > > > > > The enum values (which are not documented in KMS docs, btw.) are tuples > > > of color space + color model, e.g. on Intel: > > > > > > "Colorspace": enum {Default, SMPTE_170M_YCC, BT709_YCC, XVYCC_601, > > > XVYCC_709, SYCC_601, opYCC_601, opRGB, BT2020_CYCC, BT2020_RGB, > > > BT2020_YCC, DCI-P3_RGB_D65, DCI-P3_RGB_Theater} > > > > The accepted values are just what the CTA-861/DP specs > > allow us to transmit in he infoframe/SDP/MSA. > > Sure, but I mean the KMS doc a) does not refer to any standard, and b) > does not even list what the possible values could be. Seems like something that can be remedied with a patch. > > > > > > > > Reading the KMS docs from > > > https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-connector-properties > > > they say: > > > > > > > Basically the expectation from userspace is: > > > > > > > > Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink > > > > colorspace > > > > > > > > Set this new property to let the sink know what it converted > > > > the CRTC output to. > > > > > > > > This property is just to inform sink what colorspace source > > > > is trying to drive. > > > > > > However, where does userspace program the actual conversion from RGB to > > > NNN? Is it expected to be embedded in CTM? > > > > > > Or does setting "Colorspace" imply some additional automatic > > > conversion? If so, where does it happen and how is it chosen? > > > > > > > For just the plymouth case I guess the easiest thing would be to > > > > just clamp "max bpc" to the current value. The problem with that > > > > is that we'd potentially break existing userspace. Eg. I don't think > > > > the modesetting ddx or perhaps even most of the wayland compositors > > > > set "max bpc" at all. So we'd need to roll out a bunch of userspace > > > > updates first. And the "current bpc" prop idea would also have this > > > > same problem since plymouth would just clamp "max bpc" based on the > > > > current bpc before the real userspace starts. > > > > > > True, but I believe once color management spreads through Wayland, so > > > will KMS clients also learn to set it. > > > > Sure. But my point is that if we want to change how the "max bpc" > > works I think we need to roll out the userspace stuff first so that > > we at least can tell the user "please update you userspace to release x" > > when they hit the regression. > > Sorry, I lost track on who is suggesting to change what. > > I thought we agreed that "max bpc" means limiting link bpc to at most > that value, but the driver will automatically pick a lower value if > that makes the requested video mode work (and in lack of new KMS > properties). > > I have no desire that change that. I also think the Plymouth issue is > not fully fixable without some new KMS property, and until then the > only thing Plymouth could do is to smash "max bpc" always to 8 - which > mostly stops being a problem once display servers learn to handle "max > bpc". There's no real differene between the kernel automagically clamping "max bpc" to the current BIOS programmed value vs. plymouth doing it. Either approach will break deep color support for current userspace which doesn't reset "max bpc" back to the max. > > However, when new KMS properties are introduced, I very much like to > promote the two property setup for anything that could be a) set to > "auto", or b) be changed by the kernel *and* userspace. > > > Thanks, > pq -- Ville Syrjälä Intel
