On Thu, Jun 2, 2022 at 6:40 PM Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > 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. So what are we supposed to do with the Colorspace property other than setting it to default? > > > > > > > > > > > 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 >
