Initial Contributions to Wayland

2023-11-07 Thread Forcha Pearl
Hello members,
I'm Forcha Pearl, a final-year Computer Science postgraduate.
I'm good at Python, C++,C and JavaScript languages and have completed a few
projects,
excited to explore open source by contributing and learning.

Could anyone kindly assist me in making my initial contribution to X.ORG
 foundation by suggesting beginner-friendly issues?


Re: Exploring the Need for a Standardized Window Management API within or alongside Wayland

2023-11-07 Thread Michael Tretter
On Fri, 03 Nov 2023 09:33:10 -0700, Wayne Sherman wrote:
> Is a unified approach for window management in the Wayland ecosystem needed?
> (i.e how can the end user or system designer control the size,
> position, and layer of top level application windows).

I certainly see the use cases and probably the need for a unified approach for
more control over window geometry and stacking [0]. There already exist a few
Wayland shells and workarounds that address this issue.

[...]
> Certain applications and workflows, particularly those involving
> complex window arrangements, dynamic window resizing, and precise
> positioning (e.g., tiling window managers, graphical editing tools,
> and presentation software), can greatly benefit from having a measure
> of control over their window geometry and stacking. Currently,
> attempts at providing this are being done by a variety of
> compositor-specific protocols and extensions, which can lead to
> fragmentation and inconsistent user and developer experiences across
> different environments.  A standard API or protocol would only serve
> the needs of these applications and would also aim to prevent the
> proliferation of inconsistent and ad hoc solutions.

I think it is important to split the discussion between client applications
that would like to position their own windows, and compositors/shells that
implement predefined (by the user or system developer) rules.

Pretty much all use cases that come to my mind could (and should) be addressed
in the compositor/shell. Various compositors and shells already implement one
or another mechanism to kind of define how the windows are arranged. Usually
these are pretty limited or specific to the compositors needs.  Working
towards something unified may be a nice goal, but having something that
doesn't feel like like an ad-hoc solution or hack would be a nice intermediate
step. This would address the embedded use cases and some kind of
compositor/shell automation.

On the other hand, I think adding a mechanism for applications to control
their own windows is not reasonable. Such a mechanism either becomes too
complex and fragile to use, or to restricted for a lot of use cases.

Michael

[0] https://www.youtube.com/watch?v=qK2Emqp9t0g&t=14244s


Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Pekka Paalanen
On Mon, 6 Nov 2023 11:24:50 -0500
Harry Wentland  wrote:

> On 2023-10-20 06:17, Pekka Paalanen wrote:
> > On Thu, 19 Oct 2023 10:56:29 -0400
> > Harry Wentland  wrote:
> >   
> >> On 2023-09-13 07:29, Pekka Paalanen wrote:  
> >>> On Fri, 8 Sep 2023 11:02:26 -0400
> >>> Harry Wentland  wrote:
> >>> 
>  Signed-off-by: Harry Wentland   
> > 
> > ...
> >   
>  +COLOR_PIPELINE Plane Property
>  +=
>  +
>  +Because we don't have existing KMS color properties in the pre-blending
>  +portion of display pipelines (i.e. on drm_planes) we are introducing
>  +color pipelines here first. Eventually we'll want to use the same
>  +concept for the post-blending portion, i.e. drm_crtcs.
> >>>
> >>> This paragraph might fit better in a cover letter.
> >>> 
>  +
>  +Color Pipelines are created by a driver and advertised via a new
>  +COLOR_PIPELINE enum property on each plane. Values of the property
>  +always include '0', which is the default and means all color processing
>  +is disabled. Additional values will be the object IDs of the first
>  +drm_colorop in a pipeline. A driver can create and advertise none, one,
>  +or more possible color pipelines. A DRM client will select a color
>  +pipeline by setting the COLOR PIPELINE to the respective value.
>  +
>  +In the case where drivers have custom support for pre-blending color
>  +processing those drivers shall reject atomic commits that are trying to
>  +set both the custom color properties, as well as the COLOR_PIPELINE
> >>>
> >>> s/set/use/ because one of them could be carried-over state from
> >>> previous commits while not literally set in this one.
> >>> 
>  +property.
>  +
>  +An example of a COLOR_PIPELINE property on a plane might look like 
>  this::
>  +
>  +Plane 10
>  +├─ "type": immutable enum {Overlay, Primary, Cursor} = Primary
>  +├─ …
>  +└─ "color_pipeline": enum {0, 42, 52} = 0
> >>>
> >>> Enum values are string names. I presume the intention here is that the
> >>> strings will never need to be parsed, and the uint64_t is always equal
> >>> to the string representation, right?
> >>>
> >>> That needs a statement here. It differs from all previous uses of
> >>> enums, and e.g. requires a little bit of new API in libweston's
> >>> DRM-backend to handle since it has its own enums referring to the
> >>> string names that get mapped to the uint64_t per owning KMS object.
> >>> 
> >>
> >> I'm currently putting the DRM object ID in the "value" and use the
> >> "name" as a descriptive name.  
> > 
> > Would that string name be UAPI? I mean, if userspace hardcodes and
> > looks for that name, will that keep working? If it's descriptive then I
> > would assume not, but for every enum existing so far the string name is
> > UAPI.
> >   
> 
> Yes, it's UAPI, as that's how userspace will set the property. The value
> is still important to be able to find out which is the first colorop in
> the pipeline.

Userspace will hardcode string names, look up the KMS uint64_t
corresponding to it, and then use the uint64_t to program KMS.

But for color pipeline objects, the initial idea was that we expect
userspace to look through all available pipelines and see if any of
them can express what userspace wants. This does not need the string
name to be UAPI per se.

Of course, it is easier if userspace can be hardcoded for a specific
color pipeline, so all that matching and searching is avoided, but as a
driver developer, do you want that?

Or maybe the practical end result is the same regardless, because if a
driver removes a pipeline on specific hardware and userspace cannot
find another, that would be a kernel regression anyway.

Then again, if userspace doesn't do the matching and searching, it'll
likely struggle to work on different hardware. Driver developers would
get requests to expose this and that specific pipeline. Is that an ok
prospect?


Thanks,
pq


> >>> struct drm_mode_property_enum {
> >>>   __u64 value;
> >>>   char name[DRM_PROP_NAME_LEN];
> >>> };
> >>
> >> This works well in IGT and gives us a nice descriptive name for
> >> debugging, but I could consider changing this if it'd simplify
> >> people's lives.  
> > 
> > It's nice if we can have a description string for each pipeline, but
> > according to KMS UAPI conventions so far, userspace would look for the
> > string name. I'm worried that could backfire to the kernel.
> > 
> > Or, maybe we do want to make the string UAPI and accept that some
> > userspace might look for it rather than an arrangement of colorops?
> > 
> > Matching colorop sequences is "hard". A developer looking at pipelines,
> > picking one, and hardcoding its name as a preferred choice would be too
> > easy. "Works for my cards." IOW, if there is a useful looking string
> > name, we can be sure that someone will depend on it.
> > 
> > 
> > Than

Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Pekka Paalanen
On Mon, 6 Nov 2023 11:19:27 -0500
Harry Wentland  wrote:

> On 2023-10-20 06:36, Pekka Paalanen wrote:
> > On Thu, 19 Oct 2023 10:56:40 -0400
> > Harry Wentland  wrote:
> >   
> >> On 2023-10-10 12:13, Melissa Wen wrote:  
> >>> O 09/08, Harry Wentland wrote:
>  Signed-off-by: Harry Wentland   
> > 
> > ...
> >   
> >>> Also, with this new plane API in place, I understand that we will
> >>> already need think on how to deal with the mixing between old drm color
> >>> properties (color encoding and color range) and these new way of setting
> >>> plane color properties. IIUC, Pekka asked a related question about it
> >>> when talking about CRTC automatic RGB->YUV (?) 
> >>> 
> >>
> >> We'll still need to confirm whether we'll want to deprecate these
> >> existing properties. If we do that we'd want a client prop. Things
> >> should still work without deprecating them, if drivers just pick up
> >> after the initial encoding and range CSC.
> >>
> >> But realistically it might be better to deprecate them and turn them
> >> into explicit colorops.  
> > 
> > The existing properties would need to be explicitly reflected in the
> > new pipelines anyway, otherwise there would always be doubt at which
> > point of a pipeline the old properties apply, and they might even
> > need to change positions between pipelines.
> > 
> > I think it is simply easier to just hide all old color related
> > properties when userspace sets the client-cap to enable pipelines. The
> > problem is to make sure to hide all old properties on all drivers that
> > support the client-cap.
> > 
> > As a pipeline must be complete (describe everything that happens to
> > pixel values), it's going to be a flag day per driver.
> > 
> > Btw. the plane FB YUV->RGB conversion needs a colorop in every pipeline
> > as well. Maybe it's purely informative and non-configurable, keyed by
> > FB pixel format, but still.
> > 
> > We also need a colorop to represent sample filtering, e.g. bilinear,
> > like I think Sebastian may have mentioned in the past. Everything
> > before the sample filter happens "per tap" as Joshua Ashton put it, and
> > everything after it happens on the sample that was computed as a
> > weighted average of the filter tap inputs (texels).
> > 
> > There could be colorops other than sample filtering that operate on
> > more than one sample at a time, like blur or sharpness. There could
> > even be colorops that change the image size like adding padding that
> > the following colorop hardware requires, and then yet another colorop
> > that clips that padding away. For an example, see
> > https://lists.freedesktop.org/archives/dri-devel/2023-October/427015.html
> > 
> > If that padding and its color can affect the pipeline results of the
> > pixels near the padding (e.g. some convolution is applied with them,
> > which may be the reason why padding is necessary to begin with), then
> > it would be best to model it.
> >   
> 
> I hear you but I'm also somewhat shying away from defining this at this point.

Would you define them before the new UAPI is released though?

I agree there is no need to have them in this patch series, but I think
we'd hit the below problems if the UAPI is released without them.

> There are already too many things that need to happen and I will focus on the
> actual color blocks (LUTs, matrices) first. We'll always be able to add a new
> (read-only) colorop type to define scaling and tap behavior at any point and
> a client is free to ignore a color pipeline if it doesn't find any tap/scale
> info in it.

How would userspace know to look for tap/scale info, if there is no
upstream definition even on paper?

And the opposite case, if someone writes userspace without tap/scale
colorops, and then drivers add those, and there is no pipeline without
them, because they always exist. Would that userspace disregard all
those pipelines because it does not understand tap/scale colorops,
leaving no usable pipelines? Would that not be kernel regressing
userspace?

If the kernel keeps on exposing pipelines without the colorops, it
fails the basic promise of the whole design: that all pixel value
affecting operations are at least listed if not controllable.

How will we avoid painting ourselves in a corner?

Maybe we need a colorop for "here be dragons" documented as having
unknown and unreliable effects, until driver authors are sure that
everything has been modelled in the pipeline and there are no unknowns?
Or a flag on the pipelines, if we can have that. Then we can at least
tell when the pipeline does not fulfil the basic promise.


Thanks,
pq


pgpMFx8clGUko.pgp
Description: OpenPGP digital signature


Re: Steam Deck integrated display: saturation boosting or reduction?

2023-11-07 Thread Joshua Ashton




On 11/6/23 10:21, Pekka Paalanen wrote:

On Sat, 4 Nov 2023 13:11:02 +
Joshua Ashton  wrote:


Hello,


Hi Joshua,

thanks for replying. The reason I started this discussion is that I
would like us to come to the same page on terminology and what the math
actually means. I cannot and do not want to claim anything wrong in
your math or implementation, but I would like to describe it from a
certain point of view.


Sure, I can see your perspective -- it really depends which way you are 
looking at the color pipeline.


Speaking strictly from a math perspective we do a CTM of Hypothetical 
Display -> Native Primaries near the grey axis and smoothly decrease the 
influence of that operation near gamut edges to get the image we put on 
screen.





The existing behaviour before any of our colour work was that the native
display's primaries were being used for SDR content. (Ie. just scanning
out game's buffer directly)


Ok. That means it was using an implicit conversion from whatever the
games were emitting into the display's native primaries. Using an
(implicit) identity CTM unavoidably changes colorimetry when the source
and destination color spaces differ.


Yeah.

If you're looking at the bigger picture with that implicit conversion on 
the display side, you can say it's actually saturation reduction from 
our hypothetical display to fit onto our modest gamut internal display.


But...

If you look at the pixels we are sending to the display compared to what 
we were before -- we're increasing the saturation non-linearly, and 
that's also the result that the users will see.


The thing we are changing here with this work is what that "hypothetical 
display" is.
Previously it was the same as the internal display, but we changed that 
in order to achieve a saturation boost to compensate for the modest 
gamut of the display.


From the other end, if there was a wide gamut display attached to a 
Deck, and we wanted to display SDR content as 709 on it, I would not 
call that "saturation boosting" or a "saturation increase" myself.

Would you?

We also use the same SDR Gamut Wideness slider for HDR and Wide Gamut 
displays too fwiw.


I also don't believe either perspective is wrong -- it just depends what 
way you look at it.




If games were graded to produce BT.2020 encoding, this would amount to
extreme saturation reduction. If games were graded to produce sRGB
(perhaps implicitly, like almost all SDR apps do), this would result in
some saturation reduction.

When I say "graded", I mean something that can also happen implicitly
by a developer thinking: "it looks good on my monitor, and I'd like
everyone to see it like I do". The developer may not even think about
it. We have no knowledge of what the content was graded for, but it
still happened.


Yeah, with our "hypothetical display" we are just picking something we 
think the true intent probably was.


We actually have no knowledge of it.




Games are not submitting us any primaries for the buffers they are sending.
I mean they are saying they are sRGB so "technically 709", but
colorimetry for SDR content (outside of mastering) is very wishy-washy.


Right.


Deck Display Info:
static constexpr displaycolorimetry_t displaycolorimetry_steamdeck_spec
{
.primaries = { { 0.602f, 0.355f }, { 0.340f, 0.574f }, { 0.164f, 0.121f
} },
.white = { 0.3070f, 0.3220f },  // not D65
};

static constexpr displaycolorimetry_t displaycolorimetry_steamdeck_measured
{
.primaries = { { 0.603f, 0.349f }, { 0.335f, 0.571f }, { 0.163f, 0.115f
} },
.white = { 0.296f, 0.307f }, // not D65
};

https://github.com/ValveSoftware/gamescope/blob/master/src/color_helpers.h#L451

For the rest of this, consider displaycolorimetry_steamdeck_measured to
be what we use for the internal display.

To improve the rendering of content on the Deck's internal display with
the modest gamut, we go from the display's native primaries (sub 709) to
somewhere between the native primaries (0.0) and a hypothetical wider
gamut display (1.0) that we made up.


What do you mean by "we go from ... to ..."? What does "go" stand for
here?

Do I understand right, that you choose the target primaries from
somewhere between the display's native primaries and the below
hypothetical display's wider gamut primaries, based on end user
vibrance setting?

It's really curious if it indeed is *target* primaries for your color
transformation, given that you are still driving the internal display.


Bleh, speaking of color stuff in words is always so confusing.

When I meant go I meant in terms of the effect on the output, not the 
actual transformation. The actual mathematical transformation on the 
pixels would be the inverse of what I said for "go".


Ie. CTM of "Hypothetical display" To XYZ * XYZ To Native Primaries.




The hypothetical display's primaries were decided based by making
content look appealing:
static constexpr displaycolorimetry_t displaycolorimetry_widegamutgeneri

Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Harry Wentland



On 2023-11-07 04:38, Pekka Paalanen wrote:
> On Mon, 6 Nov 2023 11:24:50 -0500
> Harry Wentland  wrote:
> 
>> On 2023-10-20 06:17, Pekka Paalanen wrote:
>>> On Thu, 19 Oct 2023 10:56:29 -0400
>>> Harry Wentland  wrote:
>>>   
 On 2023-09-13 07:29, Pekka Paalanen wrote:  
> On Fri, 8 Sep 2023 11:02:26 -0400
> Harry Wentland  wrote:
> 
>> Signed-off-by: Harry Wentland   
>>>
>>> ...
>>>   
>> +COLOR_PIPELINE Plane Property
>> +=
>> +
>> +Because we don't have existing KMS color properties in the pre-blending
>> +portion of display pipelines (i.e. on drm_planes) we are introducing
>> +color pipelines here first. Eventually we'll want to use the same
>> +concept for the post-blending portion, i.e. drm_crtcs.
>
> This paragraph might fit better in a cover letter.
> 
>> +
>> +Color Pipelines are created by a driver and advertised via a new
>> +COLOR_PIPELINE enum property on each plane. Values of the property
>> +always include '0', which is the default and means all color processing
>> +is disabled. Additional values will be the object IDs of the first
>> +drm_colorop in a pipeline. A driver can create and advertise none, one,
>> +or more possible color pipelines. A DRM client will select a color
>> +pipeline by setting the COLOR PIPELINE to the respective value.
>> +
>> +In the case where drivers have custom support for pre-blending color
>> +processing those drivers shall reject atomic commits that are trying to
>> +set both the custom color properties, as well as the COLOR_PIPELINE
>
> s/set/use/ because one of them could be carried-over state from
> previous commits while not literally set in this one.
> 
>> +property.
>> +
>> +An example of a COLOR_PIPELINE property on a plane might look like 
>> this::
>> +
>> +Plane 10
>> +├─ "type": immutable enum {Overlay, Primary, Cursor} = Primary
>> +├─ …
>> +└─ "color_pipeline": enum {0, 42, 52} = 0
>
> Enum values are string names. I presume the intention here is that the
> strings will never need to be parsed, and the uint64_t is always equal
> to the string representation, right?
>
> That needs a statement here. It differs from all previous uses of
> enums, and e.g. requires a little bit of new API in libweston's
> DRM-backend to handle since it has its own enums referring to the
> string names that get mapped to the uint64_t per owning KMS object.
> 

 I'm currently putting the DRM object ID in the "value" and use the
 "name" as a descriptive name.  
>>>
>>> Would that string name be UAPI? I mean, if userspace hardcodes and
>>> looks for that name, will that keep working? If it's descriptive then I
>>> would assume not, but for every enum existing so far the string name is
>>> UAPI.
>>>   
>>
>> Yes, it's UAPI, as that's how userspace will set the property. The value
>> is still important to be able to find out which is the first colorop in
>> the pipeline.
> 
> Userspace will hardcode string names, look up the KMS uint64_t
> corresponding to it, and then use the uint64_t to program KMS.
> 
> But for color pipeline objects, the initial idea was that we expect
> userspace to look through all available pipelines and see if any of
> them can express what userspace wants. This does not need the string
> name to be UAPI per se.
> 
> Of course, it is easier if userspace can be hardcoded for a specific
> color pipeline, so all that matching and searching is avoided, but as a
> driver developer, do you want that?
> 
> Or maybe the practical end result is the same regardless, because if a
> driver removes a pipeline on specific hardware and userspace cannot
> find another, that would be a kernel regression anyway.
> 
> Then again, if userspace doesn't do the matching and searching, it'll
> likely struggle to work on different hardware. Driver developers would
> get requests to expose this and that specific pipeline. Is that an ok
> prospect?
> 

IMO in Linux land there will always be two types of DRM clients.

Ones that aim to work on all systems (within reasons) and use as
much of the DRM functionality in a driver-agnostic fashion. These
are things like Weston, Gnome, KDE, sway, etc., aka your "canonical
upstream project[s]" as mentioned in the "Userland Interfaces"
section of the "GPU Driver Developer's Guide".

Then there are people that build custom solutions for custom systems
with no desire or incentive to have them work in a driver-agnostic
fashion. These are often built on top of existing projects, such as
Weston, with customizations that are never designed to go upstream.

In the latter case I think it's entirely fair to hard-code the desired
color pipelines. In the former case I think that would be extremely
short-sighted and go against the driver-agnostic design philosophies
of

Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Harry Wentland



On 2023-10-25 16:16, Alex Goins wrote:
> Thank you Harry and all other contributors for your work on this. Responses
> inline -
> 

Thanks for your comments on this. Apologies for the late response.
I was focussing on the simpler responses to my patch set first and
left your last as it's the most interesting.

> On Mon, 23 Oct 2023, Pekka Paalanen wrote:
> 
>> On Fri, 20 Oct 2023 11:23:28 -0400
>> Harry Wentland  wrote:
>>
>>> On 2023-10-20 10:57, Pekka Paalanen wrote:
 On Fri, 20 Oct 2023 16:22:56 +0200
 Sebastian Wick  wrote:
   
> Thanks for continuing to work on this!
>
> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:  
>> v2:
>>  - Update colorop visualizations to match reality (Sebastian, Alex Hung)
>>  - Updated wording (Pekka)
>>  - Change BYPASS wording to make it non-mandatory (Sebastian)
>>  - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane Property
>>section (Pekka)
>>  - Use PQ EOTF instead of its inverse in Pipeline Programming example 
>> (Melissa)
>>  - Add "Driver Implementer's Guide" section (Pekka)
>>  - Add "Driver Forward/Backward Compatibility" section (Sebastian, Pekka)

 ...

>> +An example of a drm_colorop object might look like one of these::
>> +
>> +/* 1D enumerated curve */
>> +Color operation 42
>> +├─ "TYPE": immutable enum {1D enumerated curve, 1D LUT, 3x3 matrix, 
>> 3x4 matrix, 3D LUT, etc.} = 1D enumerated curve
>> +├─ "BYPASS": bool {true, false}
>> +├─ "CURVE_1D_TYPE": enum {sRGB EOTF, sRGB inverse EOTF, PQ EOTF, PQ 
>> inverse EOTF, …}
>> +└─ "NEXT": immutable color operation ID = 43
> 
> I know these are just examples, but I would also like to suggest the 
> possibility
> of an "identity" CURVE_1D_TYPE. BYPASS = true might get different results
> compared to setting an identity in some cases depending on the hardware. See
> below for more on this, RE: implicit format conversions.
> 
> Although NVIDIA hardware doesn't use a ROM for enumerated curves, it came up 
> in
> offline discussions that it would nonetheless be helpful to expose enumerated
> curves in order to hide the vendor-specific complexities of programming
> segmented LUTs from clients. In that case, we would simply refer to the
> enumerated curve when calculating/choosing segmented LUT entries.
> 
> Another thing that came up in offline discussions is that we could use 
> multiple
> color operations to program a single operation in hardware. As I understand 
> it,
> AMD has a ROM-defined LUT, followed by a custom 4K entry LUT, followed by an
> "HDR Multiplier". On NVIDIA we don't have these as separate hardware stages, 
> but
> we could combine them into a singular LUT in software, such that you can 
> combine
> e.g. segmented PQ EOTF with night light. One caveat is that you will lose
> precision from the custom LUT where it overlaps with the linear section of the
> enumerated curve, but that is unavoidable and shouldn't be an issue in most
> use-cases.
> 

FWIW, for the most part we don't have ROMs followed by custom LUTs. We have
either a ROM-based HW block or a segmented programmable LUT. In the case of the
former we will only expose named transfer functions. In the case of the latter
we expose a named TF, followed by custom LUT and merge them into one segmented
LUT.

> Actually, the current examples in the proposal don't include a multiplier 
> color
> op, which might be useful. For AMD as above, but also for NVIDIA as the
> following issue arises:
> 

The current examples are only examples. A multiplier coloro opwould make a lot
of sense.

> As discussed further below, the NVIDIA "degamma" LUT performs an implicit 
> fixed
> point to FP16 conversion. In that conversion, what fixed point 0x maps
> to in floating point varies depending on the source content. If it's SDR
> content, we want the max value in FP16 to be 1.0 (80 nits), subject to a
> potential boost multiplier if we want SDR content to be brighter. If it's HDR 
> PQ
> content, we want the max value in FP16 to be 125.0 (10,000 nits). My 
> assumption
> is that this is also what AMD's "HDR Multiplier" stage is used for, is that
> correct?
> 

Our PQ transfer function will also map to [0.0, 125.0] without use of the HDR
multiplier. The HDR multiplier is intended to be used to scale SDR brightness
when the user moves the SDR brightness slider in the OS.

> From the given enumerated curves, it's not clear how they would map to the
> above. Should sRGB EOTF have a max FP16 value of 1.0, and the PQ EOTF a max 
> FP16
> value of 125.0? That may work, but it tends towards the "descriptive" notion 
> of

Yes, I think we need to be clear about the output range of a named transfer
function. While AMD and NVidia map PQ to [0.0, 125.0] I could see others map
it to [0.0, 1.0] (and maybe scale sRGB down to 1/125.0 or some other value).

> assuming the source content, which may 

Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Harry Wentland



On 2023-10-26 04:57, Pekka Paalanen wrote:
> On Wed, 25 Oct 2023 15:16:08 -0500 (CDT)
> Alex Goins  wrote:
> 
>> Thank you Harry and all other contributors for your work on this. Responses
>> inline -
>>
>> On Mon, 23 Oct 2023, Pekka Paalanen wrote:
>>
>>> On Fri, 20 Oct 2023 11:23:28 -0400
>>> Harry Wentland  wrote:
>>>   
 On 2023-10-20 10:57, Pekka Paalanen wrote:  
> On Fri, 20 Oct 2023 16:22:56 +0200
> Sebastian Wick  wrote:
> 
>> Thanks for continuing to work on this!
>>
>> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:
>>> v2:
>>>  - Update colorop visualizations to match reality (Sebastian, Alex Hung)
>>>  - Updated wording (Pekka)
>>>  - Change BYPASS wording to make it non-mandatory (Sebastian)
>>>  - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane Property
>>>section (Pekka)
>>>  - Use PQ EOTF instead of its inverse in Pipeline Programming example 
>>> (Melissa)
>>>  - Add "Driver Implementer's Guide" section (Pekka)
>>>  - Add "Driver Forward/Backward Compatibility" section (Sebastian, 
>>> Pekka)  
>
> ...
>  
>>> +An example of a drm_colorop object might look like one of these::
>>> +
>>> +/* 1D enumerated curve */
>>> +Color operation 42
>>> +├─ "TYPE": immutable enum {1D enumerated curve, 1D LUT, 3x3 
>>> matrix, 3x4 matrix, 3D LUT, etc.} = 1D enumerated curve
>>> +├─ "BYPASS": bool {true, false}
>>> +├─ "CURVE_1D_TYPE": enum {sRGB EOTF, sRGB inverse EOTF, PQ EOTF, 
>>> PQ inverse EOTF, …}
>>> +└─ "NEXT": immutable color operation ID = 43  
>>
>> I know these are just examples, but I would also like to suggest the 
>> possibility
>> of an "identity" CURVE_1D_TYPE. BYPASS = true might get different results
>> compared to setting an identity in some cases depending on the hardware. See
>> below for more on this, RE: implicit format conversions.
>>
>> Although NVIDIA hardware doesn't use a ROM for enumerated curves, it came up 
>> in
>> offline discussions that it would nonetheless be helpful to expose enumerated
>> curves in order to hide the vendor-specific complexities of programming
>> segmented LUTs from clients. In that case, we would simply refer to the
>> enumerated curve when calculating/choosing segmented LUT entries.
> 
> That's a good idea.
> 
>> Another thing that came up in offline discussions is that we could use 
>> multiple
>> color operations to program a single operation in hardware. As I understand 
>> it,
>> AMD has a ROM-defined LUT, followed by a custom 4K entry LUT, followed by an
>> "HDR Multiplier". On NVIDIA we don't have these as separate hardware stages, 
>> but
>> we could combine them into a singular LUT in software, such that you can 
>> combine
>> e.g. segmented PQ EOTF with night light. One caveat is that you will lose
>> precision from the custom LUT where it overlaps with the linear section of 
>> the
>> enumerated curve, but that is unavoidable and shouldn't be an issue in most
>> use-cases.
> 
> Indeed.
> 
>> Actually, the current examples in the proposal don't include a multiplier 
>> color
>> op, which might be useful. For AMD as above, but also for NVIDIA as the
>> following issue arises:
>>
>> As discussed further below, the NVIDIA "degamma" LUT performs an implicit 
>> fixed
>> point to FP16 conversion. In that conversion, what fixed point 0x 
>> maps
>> to in floating point varies depending on the source content. If it's SDR
>> content, we want the max value in FP16 to be 1.0 (80 nits), subject to a
>> potential boost multiplier if we want SDR content to be brighter. If it's 
>> HDR PQ
>> content, we want the max value in FP16 to be 125.0 (10,000 nits). My 
>> assumption
>> is that this is also what AMD's "HDR Multiplier" stage is used for, is that
>> correct?
> 
> It would be against the UAPI design principles to tag content as HDR or
> SDR. What you can do instead is to expose a colorop with a multiplier of
> 1.0 or 125.0 to match your hardware behaviour, then tell your hardware
> that the input is SDR or HDR to get the expected multiplier. You will
> never know what the content actually is, anyway.
> 
> Of course, if we want to have a arbitrary multiplier colorop that is
> somewhat standard, as in, exposed by many drivers to ease userspace
> development, you can certainly use any combination of your hardware
> features you need to realize the UAPI prescribed mathematical operation.
> 
> Since we are talking about floating-point in hardware, a multiplier
> does not significantly affect precision.
> 
> In order to mathematically define all colorops, I believe it is
> necessary to define all colorops in terms of floating-point values (as
> in math), even if they operate on fixed-point or integer. By this I
> mean that if the input is 8 bpc unsigned integer pixel format for
> instance, 0 raw pixel channel value is mapped to 0.0 and 255 is mapped
> to 1.0, 

Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Harry Wentland



On 2023-10-26 13:30, Sebastian Wick wrote:
> On Thu, Oct 26, 2023 at 11:57:47AM +0300, Pekka Paalanen wrote:
>> On Wed, 25 Oct 2023 15:16:08 -0500 (CDT)
>> Alex Goins  wrote:
>>
>>> Thank you Harry and all other contributors for your work on this. Responses
>>> inline -
>>>
>>> On Mon, 23 Oct 2023, Pekka Paalanen wrote:
>>>
 On Fri, 20 Oct 2023 11:23:28 -0400
 Harry Wentland  wrote:
   
> On 2023-10-20 10:57, Pekka Paalanen wrote:  
>> On Fri, 20 Oct 2023 16:22:56 +0200
>> Sebastian Wick  wrote:
>> 
>>> Thanks for continuing to work on this!
>>>
>>> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:

snip

>>
>> I think we also need a definition of "informational".
>>
>> Counter-example 1: a colorop that represents a non-configurable
>
> Not sure what's "counter" for these examples?
>   
>> YUV<->RGB conversion. Maybe it determines its operation from FB pixel
>> format. It cannot be set to bypass, it cannot be configured, and it
>> will alter color values.  
>>>
>>> Would it be reasonable to expose this is a 3x4 matrix with a read-only blob 
>>> and
>>> no BYPASS property? I already brought up a similar idea at the XDC HDR 
>>> Workshop
>>> based on the principle that read-only blobs could be used to express some 
>>> static
>>> pipeline elements without the need to define a new type, but got mixed 
>>> opinions.
>>> I think this demonstrates the principle further, as clients could detect 
>>> this
>>> programmatically instead of having to special-case the informational 
>>> element.
>>
> 
> I'm all for exposing fixed color ops but I suspect that most of those
> follow some standard and in those cases instead of exposing the matrix
> values one should prefer to expose a named matrix (e.g. BT.601, BT.709,
> BT.2020).
> 

Agreed.

> As a general rule: always expose the highest level description. Going
> from a name to exact values is trivial, going from values to a name is
> much harder.
> 
>> If the blob depends on the pixel format (i.e. the driver automatically
>> chooses a different blob per pixel format), then I think we would need
>> to expose all the blobs and how they correspond to pixel formats.
>> Otherwise ok, I guess.
>>
>> However, do we want or need to make a color pipeline or colorop
>> conditional on pixel formats? For example, if you use a YUV 4:2:0 type
>> of pixel format, then you must use this pipeline and not any other. Or
>> floating-point type of pixel format. I did not anticipate this before,
>> I assumed that all color pipelines and colorops are independent of the
>> framebuffer pixel format. A specific colorop might have a property that
>> needs to agree with the framebuffer pixel format, but I didn't expect
>> further limitations.
> 
> We could simply fail commits when the pipeline and pixel format don't
> work together. We'll probably need some kind of ingress no-op node
> anyway and maybe could list pixel formats there if required to make it
> easier to find a working configuration.
> 

The problem with failing commits is that user-space has no idea why it
failed. If this means that userspace falls back to SW composition for
NV12 and P010 it would avoid HW offloading in one of the most important
use-cases on AMD HW for power-saving purposes.

snip

>>> Despite being programmable, the LUTs are updated in a manner that is less
>>> efficient as compared to e.g. the non-static "degamma" LUT. Would it be 
>>> helpful
>>> if there was some way to tag operations according to their performance,
>>> for example so that clients can prefer a high performance one when they
>>> intend to do an animated transition? I recall from the XDC HDR workshop
>>> that this is also an issue with AMD's 3DLUT, where updates can be too
>>> slow to animate.
>>
>> I can certainly see such information being useful, but then we need to
>> somehow quantize the performance.
>>
>> What I was left puzzled about after the XDC workshop is that is it
>> possible to pre-load configurations in the background (slow), and then
>> quickly switch between them? Hardware-wise I mean.
> 
> We could define that pipelines with a lower ID are to be preferred over
> higher IDs.
> 
> The issue is that if programming a pipeline becomes too slow to be
> useful it probably should just not be made available to user space.
> 
> The prepare-commit idea for blob properties would help to make the
> pipelines usable again, but until then it's probably a good idea to just
> not expose those pipelines.
> 

It's a bit of a judgment call what's too slow, though. The value of having
a HW colorop might outweigh the cost of the programming time for some
compositors but not for others.

Harry

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



Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Harry Wentland



On 2023-10-26 15:25, Alex Goins wrote:
> On Thu, 26 Oct 2023, Sebastian Wick wrote:
> 
>> On Thu, Oct 26, 2023 at 11:57:47AM +0300, Pekka Paalanen wrote:
>>> On Wed, 25 Oct 2023 15:16:08 -0500 (CDT)
>>> Alex Goins  wrote:
>>>
 Thank you Harry and all other contributors for your work on this. Responses
 inline -

 On Mon, 23 Oct 2023, Pekka Paalanen wrote:

> On Fri, 20 Oct 2023 11:23:28 -0400
> Harry Wentland  wrote:
>
>> On 2023-10-20 10:57, Pekka Paalanen wrote:
>>> On Fri, 20 Oct 2023 16:22:56 +0200
>>> Sebastian Wick  wrote:
>>>
 Thanks for continuing to work on this!

 On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:

snip

>>>
>>> If we look at BT.2100, there is no such encoding even mentioned where
>>> 125.0 would correspond to 10k cd/m². That 125.0 convention already has
>>> a built-in assumption what the color spaces are and what the conversion
>>> is aiming to do. IOW, I would say that choice is opinionated from the
>>> start. The multiplier in BT.2100 is always 1.
> 
> Be that as it may, the convention of FP16 125.0 corresponding to 10k nits is
> baked in our hardware, so it's unavoidable at least for NVIDIA pipelines.
> 

Yeah, that's not just NVidia, it's basically the same for AMD. Though I
think we can work without that assumption, but the PQ TF you get from AMD
will map to [0.0, 125.0].

snip

>>
>> We could simply fail commits when the pipeline and pixel format don't
>> work together. We'll probably need some kind of ingress no-op node
>> anyway and maybe could list pixel formats there if required to make it
>> easier to find a working configuration.
> 
> Yeah, we could, but having to figure that out through trial and error would be
> unfortunate. Per above, it might be easiest to just tag pipelines with a pixel
> format instead of trying to include the pixel format conversion as a color op.
> 

Agreed, We've been looking at libliftoff a bit but one of the problem is
that it does a lot of atomic checks to figure out an optimal HW plane
configuration and we run out of time budget before we're able to check
all options.

Atomic check failure is really not well suited for this stuff.


>>> "Without the need to define a new type" is something I think we need to
>>> consider case by case. I have a hard time giving a general opinion.
>>>
>>>
>>> Counter-example 2: image size scaling colorop. It might not be
>>> configurable, it is controlled by the plane CRTC_* and SRC_*
>>> properties. You still need to understand what it does, so you can
>>> arrange the scaling to work correctly. (Do not want to scale an image
>>> with PQ-encoded values as Josh demonstrated in XDC.)
>>>
>>
>> IMO the position of the scaling operation is the thing that's important
>> here as the color pipeline won't define scaling properties.

 I agree that blending should ideally be done in linear space, and I 
 remember
 that from Josh's presentation at XDC, but I don't recall the same being 
 said for
 scaling. In fact, the NVIDIA pre-blending scaler exists in a stage of the
 pipeline that is meant to be in PQ space (more on this below), and that was
 found to achieve better results at HDR/SDR boundaries. Of course, this only
 bolsters the argument that it would be helpful to have an informational 
 "scaler"
 element to understand at which stage scaling takes place.
>>>
>>> Both blending and scaling are fundamentally the same operation: you
>>> have two or more source colors (pixels), and you want to compute a
>>> weighted average of them following what happens in nature, that is,
>>> physics, as that is what humans are used to.
>>>
>>> Both blending and scaling will suffer from the same problems if the
>>> operation is performed on not light-linear values. The result of the
>>> weighted average does not correspond to physics.
>>>
>>> The problem may be hard to observe with natural imagery, but Josh's
>>> example shows it very clearly. Maybe that effect is sometimes useful
>>> for some imagery in some use cases, but it is still an accidental
>>> side-effect. You might get even better results if you don't rely on
>>> accidental side-effects but design a separate operation for the exact
>>> goal you have.
>>>
>>> Mind, by scaling we mean changing image size. Not scaling color values.
>>>
> 
> Fair enough, but it might not always be a choice given the hardware.
> 

I'm thinking of this as an information element, not a programmable.
Some HW could define this as programmable, but I probably wouldn't
on AMD HW.

snip

>>>
>>> What I was left puzzled about after the XDC workshop is that is it
>>> possible to pre-load configurations in the background (slow), and then
>>> quickly switch between them? Hardware-wise I mean.
> 
> This works fine for our "fast" LUTs, you just point them to a surface in video
> memory and they flip to it. You could keep multiple s

Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Harry Wentland



On 2023-11-04 19:01, Christopher Braga wrote:
> Just want to loop back to before we branched off deeper into the programming 
> performance talk
> 
> On 10/26/2023 3:25 PM, Alex Goins wrote:
>> On Thu, 26 Oct 2023, Sebastian Wick wrote:
>>
>>> On Thu, Oct 26, 2023 at 11:57:47AM +0300, Pekka Paalanen wrote:
 On Wed, 25 Oct 2023 15:16:08 -0500 (CDT)
 Alex Goins  wrote:

> Thank you Harry and all other contributors for your work on this. 
> Responses
> inline -
>
> On Mon, 23 Oct 2023, Pekka Paalanen wrote:
>
>> On Fri, 20 Oct 2023 11:23:28 -0400
>> Harry Wentland  wrote:
>>
>>> On 2023-10-20 10:57, Pekka Paalanen wrote:
 On Fri, 20 Oct 2023 16:22:56 +0200
 Sebastian Wick  wrote:

> Thanks for continuing to work on this!
>
> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:

snip

> Actually, the current examples in the proposal don't include a multiplier 
> color
> op, which might be useful. For AMD as above, but also for NVIDIA as the
> following issue arises:
>
> As discussed further below, the NVIDIA "degamma" LUT performs an implicit 
> fixed
> 
> If possible, let's declare this as two blocks. One that informatively 
> declares the conversion is present, and another for the de-gamma. This will 
> help with block-reuse between vendors.
> 
> point to FP16 conversion. In that conversion, what fixed point 0x 
> maps
> to in floating point varies depending on the source content. If it's SDR
> content, we want the max value in FP16 to be 1.0 (80 nits), subject to a
> potential boost multiplier if we want SDR content to be brighter. If it's 
> HDR PQ
> content, we want the max value in FP16 to be 125.0 (10,000 nits). My 
> assumption
> is that this is also what AMD's "HDR Multiplier" stage is used for, is 
> that
> correct?

 It would be against the UAPI design principles to tag content as HDR or
 SDR. What you can do instead is to expose a colorop with a multiplier of
 1.0 or 125.0 to match your hardware behaviour, then tell your hardware
 that the input is SDR or HDR to get the expected multiplier. You will
 never know what the content actually is, anyway.
>>
>> Right, I didn't mean to suggest that we should tag content as HDR or SDR in 
>> the
>> UAPI, just relating to the end result in the pipe, ultimately it would be
>> determined by the multiplier color op.
>>
> 
> A multiplier could work but we would should give OEMs the option to either 
> make it "informative" and fixed by the hardware, or fully configurable. With 
> the Qualcomm pipeline how we absorb FP16 pixel buffers, as well as how we 
> convert them to fixed point data actually has a dependency on the desired 
> de-gamma and gamma processing. So for an example:
> 
> If a source pixel buffer is scRGB encoded FP16 content we would expect input 
> pixel content to be up to 7.5, with the IGC output reaching 125 as in the 
> NVIDIA case. Likewise gamma 2.2 encoded FP16 content would be 0-1 in and 0-1 
> out.
> 
> So in the Qualcomm case the expectations are fixed depending on the use case.
> 
> It is sounding to me like we would need to be able to declare three things 
> here:
> 1. Value range expectations *into* the de-gamma block. A multiplier wouldn't 
> work here because it would be more of a clipping operation. I guess we would 
> have to add an explicit clamping block as well.
> 2. What the value range expectations  at the *output* of de-gamma processing 
> block. Also covered by using another multiplier block.
> 3. Value range expectations *into* a gamma processing block. This should be 
> covered by declaring a multiplier post-csc, but only assuming CSC output is 
> normalized in the desired value range. A clamping block would be preferable 
> because it describes what happens when it isn't.
> 

What about adding informational input and output range properties
to colorops? I think Intel's PWL definitions had something like
that, but I'd have to take a look at that again. While I'm not
in favor of defining segmented LUTs at the uAPI the input/output
ranges seem to be something of value.

> All this is do-able, but it seems like it would require the definition of 
> multiple color pipelines to expose the different limitations for color block 
> configuration combinations. Additionally, would it be easy for user space to 
> find the right pipeline?
> 

I'm also a little concerned that some of these proposals mean we'd
have to expose an inordinate number of color pipelines and color
pipeline selection becomes difficult and error prone.

snip

 Given that elements like various kinds of look-up tables inherently
 assume that the domain is [0.0, 1.0] (because the it is a table that
 has a beginning and an end, and the usual convention is that the
 beginning is zero and the end is one), I think it is best to stick to
>

Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Harry Wentland



On 2023-11-07 04:55, Pekka Paalanen wrote:
> On Mon, 6 Nov 2023 11:19:27 -0500
> Harry Wentland  wrote:
> 
>> On 2023-10-20 06:36, Pekka Paalanen wrote:
>>> On Thu, 19 Oct 2023 10:56:40 -0400
>>> Harry Wentland  wrote:
>>>   
 On 2023-10-10 12:13, Melissa Wen wrote:  
> O 09/08, Harry Wentland wrote:
>> Signed-off-by: Harry Wentland   
>>>
>>> ...
>>>   
> Also, with this new plane API in place, I understand that we will
> already need think on how to deal with the mixing between old drm color
> properties (color encoding and color range) and these new way of setting
> plane color properties. IIUC, Pekka asked a related question about it
> when talking about CRTC automatic RGB->YUV (?) 
> 

 We'll still need to confirm whether we'll want to deprecate these
 existing properties. If we do that we'd want a client prop. Things
 should still work without deprecating them, if drivers just pick up
 after the initial encoding and range CSC.

 But realistically it might be better to deprecate them and turn them
 into explicit colorops.  
>>>
>>> The existing properties would need to be explicitly reflected in the
>>> new pipelines anyway, otherwise there would always be doubt at which
>>> point of a pipeline the old properties apply, and they might even
>>> need to change positions between pipelines.
>>>
>>> I think it is simply easier to just hide all old color related
>>> properties when userspace sets the client-cap to enable pipelines. The
>>> problem is to make sure to hide all old properties on all drivers that
>>> support the client-cap.
>>>
>>> As a pipeline must be complete (describe everything that happens to
>>> pixel values), it's going to be a flag day per driver.
>>>
>>> Btw. the plane FB YUV->RGB conversion needs a colorop in every pipeline
>>> as well. Maybe it's purely informative and non-configurable, keyed by
>>> FB pixel format, but still.
>>>
>>> We also need a colorop to represent sample filtering, e.g. bilinear,
>>> like I think Sebastian may have mentioned in the past. Everything
>>> before the sample filter happens "per tap" as Joshua Ashton put it, and
>>> everything after it happens on the sample that was computed as a
>>> weighted average of the filter tap inputs (texels).
>>>
>>> There could be colorops other than sample filtering that operate on
>>> more than one sample at a time, like blur or sharpness. There could
>>> even be colorops that change the image size like adding padding that
>>> the following colorop hardware requires, and then yet another colorop
>>> that clips that padding away. For an example, see
>>> https://lists.freedesktop.org/archives/dri-devel/2023-October/427015.html
>>>
>>> If that padding and its color can affect the pipeline results of the
>>> pixels near the padding (e.g. some convolution is applied with them,
>>> which may be the reason why padding is necessary to begin with), then
>>> it would be best to model it.
>>>   
>>
>> I hear you but I'm also somewhat shying away from defining this at this 
>> point.
> 
> Would you define them before the new UAPI is released though?
> 
> I agree there is no need to have them in this patch series, but I think
> we'd hit the below problems if the UAPI is released without them.
> 
>> There are already too many things that need to happen and I will focus on the
>> actual color blocks (LUTs, matrices) first. We'll always be able to add a new
>> (read-only) colorop type to define scaling and tap behavior at any point and
>> a client is free to ignore a color pipeline if it doesn't find any tap/scale
>> info in it.
> 
> How would userspace know to look for tap/scale info, if there is no
> upstream definition even on paper?
> 

So far OSes did not care about this. Whether that's good or bad is
something everyone can answer for themselves.

If you write a compositor and really need this you can just ignore
color pipelines that don't have this, i.e., you'll probably want
to wait with implementing color pipeline support until you have what
you need from DRM/KMS.

This is not to say I don't want to have support for Weston. But I'm
wondering if we place too much importance on getting every little
thing figured out whereas we could be making forward progress and
address more aspects of a color pipeline in the future. There is a
reason gamescope has made a huge difference in driving the color
management work forward.

> And the opposite case, if someone writes userspace without tap/scale
> colorops, and then drivers add those, and there is no pipeline without
> them, because they always exist. Would that userspace disregard all
> those pipelines because it does not understand tap/scale colorops,
> leaving no usable pipelines? Would that not be kernel regressing
> userspace?
> 

The simple solution is to leave previously advertised pipelines
untouched and add a new version that does include scaling information.

> If the kernel keeps on exposing pipeline

Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed

2023-11-07 Thread Sebastian Wick
On Tue, Nov 07, 2023 at 11:52:11AM -0500, Harry Wentland wrote:
> 
> 
> On 2023-10-26 13:30, Sebastian Wick wrote:
> > On Thu, Oct 26, 2023 at 11:57:47AM +0300, Pekka Paalanen wrote:
> >> On Wed, 25 Oct 2023 15:16:08 -0500 (CDT)
> >> Alex Goins  wrote:
> >>
> >>> Thank you Harry and all other contributors for your work on this. 
> >>> Responses
> >>> inline -
> >>>
> >>> On Mon, 23 Oct 2023, Pekka Paalanen wrote:
> >>>
>  On Fri, 20 Oct 2023 11:23:28 -0400
>  Harry Wentland  wrote:
>    
> > On 2023-10-20 10:57, Pekka Paalanen wrote:  
> >> On Fri, 20 Oct 2023 16:22:56 +0200
> >> Sebastian Wick  wrote:
> >> 
> >>> Thanks for continuing to work on this!
> >>>
> >>> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:
> 
> snip
> 
> >>
> >> I think we also need a definition of "informational".
> >>
> >> Counter-example 1: a colorop that represents a non-configurable
> >
> > Not sure what's "counter" for these examples?
> >   
> >> YUV<->RGB conversion. Maybe it determines its operation from FB pixel
> >> format. It cannot be set to bypass, it cannot be configured, and it
> >> will alter color values.  
> >>>
> >>> Would it be reasonable to expose this is a 3x4 matrix with a read-only 
> >>> blob and
> >>> no BYPASS property? I already brought up a similar idea at the XDC HDR 
> >>> Workshop
> >>> based on the principle that read-only blobs could be used to express some 
> >>> static
> >>> pipeline elements without the need to define a new type, but got mixed 
> >>> opinions.
> >>> I think this demonstrates the principle further, as clients could detect 
> >>> this
> >>> programmatically instead of having to special-case the informational 
> >>> element.
> >>
> > 
> > I'm all for exposing fixed color ops but I suspect that most of those
> > follow some standard and in those cases instead of exposing the matrix
> > values one should prefer to expose a named matrix (e.g. BT.601, BT.709,
> > BT.2020).
> > 
> 
> Agreed.
> 
> > As a general rule: always expose the highest level description. Going
> > from a name to exact values is trivial, going from values to a name is
> > much harder.
> > 
> >> If the blob depends on the pixel format (i.e. the driver automatically
> >> chooses a different blob per pixel format), then I think we would need
> >> to expose all the blobs and how they correspond to pixel formats.
> >> Otherwise ok, I guess.
> >>
> >> However, do we want or need to make a color pipeline or colorop
> >> conditional on pixel formats? For example, if you use a YUV 4:2:0 type
> >> of pixel format, then you must use this pipeline and not any other. Or
> >> floating-point type of pixel format. I did not anticipate this before,
> >> I assumed that all color pipelines and colorops are independent of the
> >> framebuffer pixel format. A specific colorop might have a property that
> >> needs to agree with the framebuffer pixel format, but I didn't expect
> >> further limitations.
> > 
> > We could simply fail commits when the pipeline and pixel format don't
> > work together. We'll probably need some kind of ingress no-op node
> > anyway and maybe could list pixel formats there if required to make it
> > easier to find a working configuration.
> > 
> 
> The problem with failing commits is that user-space has no idea why it
> failed. If this means that userspace falls back to SW composition for
> NV12 and P010 it would avoid HW offloading in one of the most important
> use-cases on AMD HW for power-saving purposes.

Exposing which pixel formats work with a pipeline should be
uncontroversial, and so should be an informative scaler op.

Both can be added without a problem at a later time, so let's not make
any of that mandatory for the first version. One step after the other.

> 
> snip
> 
> >>> Despite being programmable, the LUTs are updated in a manner that is less
> >>> efficient as compared to e.g. the non-static "degamma" LUT. Would it be 
> >>> helpful
> >>> if there was some way to tag operations according to their performance,
> >>> for example so that clients can prefer a high performance one when they
> >>> intend to do an animated transition? I recall from the XDC HDR workshop
> >>> that this is also an issue with AMD's 3DLUT, where updates can be too
> >>> slow to animate.
> >>
> >> I can certainly see such information being useful, but then we need to
> >> somehow quantize the performance.
> >>
> >> What I was left puzzled about after the XDC workshop is that is it
> >> possible to pre-load configurations in the background (slow), and then
> >> quickly switch between them? Hardware-wise I mean.
> > 
> > We could define that pipelines with a lower ID are to be preferred over
> > higher IDs.
> > 
> > The issue is that if programming a pipeline becomes too slow to be
> > useful it probably should just not be made available to user space.
> > 
> > The prepare-commit idea for blob properties would help to ma