On 2/3/23 11:00, Ville Syrjälä wrote: > On Fri, Feb 03, 2023 at 10:24:52AM -0500, Harry Wentland wrote: >> >> >> On 2/3/23 10:19, Ville Syrjälä wrote: >>> On Fri, Feb 03, 2023 at 09:39:42AM -0500, Harry Wentland wrote: >>>> >>>> >>>> On 2/3/23 07:59, Sebastian Wick wrote: >>>>> On Fri, Feb 3, 2023 at 11:40 AM Ville Syrjälä >>>>> <ville.syrjala@xxxxxxxxxxxxxxx> wrote: >>>>>> >>>>>> On Fri, Feb 03, 2023 at 02:07:44AM +0000, Joshua Ashton wrote: >>>>>>> Userspace has no way of controlling or knowing the pixel encoding >>>>>>> currently, so there is no way for it to ever get the right values here. >>>>>> >>>>>> That applies to a lot of the other values as well (they are >>>>>> explicitly RGB or YCC). The idea was that this property sets the >>>>>> infoframe/MSA/SDP value exactly, and other properties should be >>>>>> added to for use userspace to control the pixel encoding/colorspace >>>>>> conversion(if desired, or userspace just makes sure to >>>>>> directly feed in correct kind of data). >>>>> >>>>> I'm all for getting userspace control over pixel encoding but even >>>>> then the kernel always knows which pixel encoding is selected and >>>>> which InfoFrame has to be sent. Is there a reason why userspace would >>>>> want to control the variant explicitly to the wrong value? >>>>> >>>> >>>> I've asked this before but haven't seen an answer: Is there an existing >>>> upstream userspace project that makes use of this property (other than >>>> what Joshua is working on in gamescope right now)? That would help us >>>> understand the intent better. >>> >>> The intent was to control the infoframe colorimetry bits, >>> nothing more. No idea what real userspace there was, if any. >>> >>>> >>>> I don't think giving userspace explicit control over the exact infoframe >>>> values is the right thing to do. >>> >>> Only userspace knows what kind of data it's stuffing into >>> the pixels (and/or how it configures the csc units/etc.) to >>> generate them. >>> >> >> Yes, but userspace doesn't control or know whether we drive >> RGB or YCbCr on the wire. In fact, in some cases our driver >> needs to fallback to YCbCr420 for bandwidth reasons. There >> is currently no way for userspace to know that and I don't >> think it makes sense. > > People want that control as well for whatever reason. We've > been asked to allow YCbCr 4:4:4 output many times. > > The automagic 4:2:0 fallback I think is rather fundementally > incompatible with fancy color management. How would we even > know whether to use eg. BT.2020 vs. BT.709 matrix? In i915 > that stuff is just always BT.709 limited range, no questions > asked. > We use what we're telling the display, i.e., the value in the colorspace property. That way we know whether to use a BT.2020 or BT.709 matrix. I don't see how it's fundamentally incompatible with fancy color management stuff. If we start forbidding drivers from falling back to YCbCr (whether 4:4:4 or 4:2:0) we will break existing behavior on amdgpu and will see bug reports. > So I think if userspace wants real color management it's > going to have to set up the whole pipeline. And for that > we need at least one new property to control the RGB->YCbCr > conversion (or to explicitly avoid it). > > And given that the proposed patch just swept all the > non-BT.2020 issues under the rug makes me think no > one has actually come up with any kind of consistent > plan for anything else really. > Does anyone actually use the non-BT.2020 colorspace stuff? Harry >> >> Userspace needs full control of framebuffer pixel formats, >> as well as control over DEGAMMA, GAMMA, CTM color operations. >> It also needs to be able to select whether to drive the panel >> as sRGB or BT.2020/PQ but it doesn't make sense for it to >> control the pixel encoding on the wire (RGB vs YCbCr). >> >>> I really don't want a repeat of the disaster of the >>> 'Broadcast RGB' which has coupled together the infoframe >>> and automagic conversion stuff. And I think this one would >>> be about 100x worse given this property has something >>> to do with actual colorspaces as well. >>> >> >> I'm unaware of this disaster. Could you elaborate? > > The property now controls both the infoframe stuff (and > whatever super vague stuff DP has for it in MSA) and > full->limited range compression in the display pipeline. > And as a result there is no way to eg. allow already > limited range input, which is what some people wanted. > > And naturally it's all made a lot more terrible by all > the displays that fail to implement the spec correctly, > but that's another topic. >
