[PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum

[Harry Wentland]

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 at linux.intel.com> 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.
>