Re: [RFC] Plane color pipeline KMS uAPI

2023-05-11 Thread Joshua Ashton
When we are talking about being 'prescriptive' in the API, are we
outright saying we don't want to support arbitrary 3D LUTs, or are we
just offering certain algorithms to be 'executed' for a plane/crtc/etc
in the atomic API? I am confused...

There is so much stuff to do with color, that I don't think a
prescriptive API in the kernel could ever keep up with the things that
we want to be pushing from Gamescope/SteamOS. For example, we have so
many things going on, night mode, SDR gamut widening, HDR/SDR gain,
the ability to apply 'looks' for eg. invert luma or for retro looks,
enhanced contrast, tonemapping, inverse tonemapping... We also are
going to be doing a bunch of stuff with EETFs for handling out of
range HDR content for scanout.

Some of what we do is kinda standard, regular "there is a paper on
this" algorithms, and others are not.
While yes, it might be very possible to do simple things, once you
start wanting to do something 'different', that's kinda lock-in.

Whether this co-exists with arbitrary LUTs (that we definitely want
for SteamOS) or not:
I think putting a bunch of math-y stuff like this into the kernel is
probably the complete wrong approach. Everything would need to be
fixed point and it would be a huge pain in the butt to deal with on
that side.

Maybe this is a "hot take", but IMO, DRM atomic is already waaay too
much being done in the kernel space. I think making it go even further
and having it be a prescriptive color API is a complete step in the
wrong direction.

There is also the problem of... if there is a bug in the math here or
we want to add a new feature, if it's kernel side, you are locked in
to having that bug until the next release on your distro and probably
years if it's a new feature!
Updating kernels is much harder for 'enterprise' distros if it is not
mission critical. Having all of this in userspace is completely fine
however...

If you want to make some userspace prescriptive -> descriptive color
library I am all for that for general case compositors, but I don't
think I would use something like that in Gamescope.
That's not to be rude, we are just picky and want freedom to do what
we want and iterate on it easily.

I guess this all comes back to my initial point... having some
userspace to handle stuff that is either kinda or entirely vendor
specific is the right way of solving this problem :-P

- Joshie 🐸✨

On Thu, 11 May 2023 at 09:51, Karol Herbst  wrote:
>
> On Wed, May 10, 2023 at 9:59β€―AM Jonas Γ…dahl  wrote:
> >
> > On Tue, May 09, 2023 at 08:22:30PM +, Simon Ser wrote:
> > > On Tuesday, May 9th, 2023 at 21:53, Dave Airlie  wrote:
> > >
> > > > There are also other vendor side effects to having this in userspace.
> > > >
> > > > Will the library have a loader?
> > > > Will it allow proprietary plugins?
> > > > Will it allow proprietary reimplementations?
> > > > What will happen when a vendor wants distros to ship their
> > > > proprietary fork of said library?
> > > >
> > > > How would NVIDIA integrate this with their proprietary stack?
> > >
> > > Since all color operations exposed by KMS are standard, the library
> > > would just be a simple one: no loader, no plugin, no proprietary pieces,
> > > etc.
> > >
> >
> > There might be pipelines/color-ops only exposed by proprietary out of
> > tree drivers; the operation types and semantics should ideally be
> > defined upstream, but the code paths would in practice be vendor
> > specific, potentially without any upstream driver using them. It should
> > be clear whether an implementation that makes such a pipeline work is in
> > scope for the upstream library.
> >
> > The same applies to the kernel; it must be clear whether pipeline
> > elements that potentially will only be exposed by out of tree drivers
> > will be acceptable upstream, at least as documented operations.
> >
>
> they aren't. All code in the kernel needs to be used by in-tree
> drivers otherwise it's fair to delete it. DRM requires any UAPI change
> to have a real open source user in space user.
>
> Nvidia knows this and they went to great lengths to fulfill this
> requirement in the past. They'll manage.
>
> >
> > Jonas
> >
>


Re: [RFC] Plane color pipeline KMS uAPI

2023-05-11 Thread Jonas Γ…dahl
On Thu, May 11, 2023 at 04:56:47PM +, Joshua Ashton wrote:
> When we are talking about being 'prescriptive' in the API, are we
> outright saying we don't want to support arbitrary 3D LUTs, or are we
> just offering certain algorithms to be 'executed' for a plane/crtc/etc
> in the atomic API? I am confused...

The 'prescriptive' idea that the RFC of this thread proposes *is* a way
to support arbitrary 3D LUTs (and other mathematical operations),
arbitrarily, in a somewhat vendored way, only that it will not be vendor
prefixed hard coded properties with specific positions in the pipeline,
but instead more or less an introspectable pipeline, describing what
kind of LUT's, Matrix multiplication (and in what order) etc a hardware
can do.

The theoretical userspace library would be the one turning descriptive
"please turn this into that" requests into the "prescriptive" color
pipeline operations. It would target general purpose compositors, but it
wouldn't be mandatory. Doing vendor specific implemantions in gamescope
would be possible; it wouldn't look like the verion that exist somewhere
now that uses a bunch of AMD_* properties, it'd look more like the
example Simon had in the initial RFC.


Jonas

> 
> There is so much stuff to do with color, that I don't think a
> prescriptive API in the kernel could ever keep up with the things that
> we want to be pushing from Gamescope/SteamOS. For example, we have so
> many things going on, night mode, SDR gamut widening, HDR/SDR gain,
> the ability to apply 'looks' for eg. invert luma or for retro looks,
> enhanced contrast, tonemapping, inverse tonemapping... We also are
> going to be doing a bunch of stuff with EETFs for handling out of
> range HDR content for scanout.
> 
> Some of what we do is kinda standard, regular "there is a paper on
> this" algorithms, and others are not.
> While yes, it might be very possible to do simple things, once you
> start wanting to do something 'different', that's kinda lock-in.
> 
> Whether this co-exists with arbitrary LUTs (that we definitely want
> for SteamOS) or not:
> I think putting a bunch of math-y stuff like this into the kernel is
> probably the complete wrong approach. Everything would need to be
> fixed point and it would be a huge pain in the butt to deal with on
> that side.
> 
> Maybe this is a "hot take", but IMO, DRM atomic is already waaay too
> much being done in the kernel space. I think making it go even further
> and having it be a prescriptive color API is a complete step in the
> wrong direction.
> 
> There is also the problem of... if there is a bug in the math here or
> we want to add a new feature, if it's kernel side, you are locked in
> to having that bug until the next release on your distro and probably
> years if it's a new feature!
> Updating kernels is much harder for 'enterprise' distros if it is not
> mission critical. Having all of this in userspace is completely fine
> however...
> 
> If you want to make some userspace prescriptive -> descriptive color
> library I am all for that for general case compositors, but I don't
> think I would use something like that in Gamescope.
> That's not to be rude, we are just picky and want freedom to do what
> we want and iterate on it easily.
> 
> I guess this all comes back to my initial point... having some
> userspace to handle stuff that is either kinda or entirely vendor
> specific is the right way of solving this problem :-P
> 
> - Joshie 🐸✨
> 
> On Thu, 11 May 2023 at 09:51, Karol Herbst  wrote:
> >
> > On Wed, May 10, 2023 at 9:59β€―AM Jonas Γ…dahl  wrote:
> > >
> > > On Tue, May 09, 2023 at 08:22:30PM +, Simon Ser wrote:
> > > > On Tuesday, May 9th, 2023 at 21:53, Dave Airlie  
> > > > wrote:
> > > >
> > > > > There are also other vendor side effects to having this in userspace.
> > > > >
> > > > > Will the library have a loader?
> > > > > Will it allow proprietary plugins?
> > > > > Will it allow proprietary reimplementations?
> > > > > What will happen when a vendor wants distros to ship their
> > > > > proprietary fork of said library?
> > > > >
> > > > > How would NVIDIA integrate this with their proprietary stack?
> > > >
> > > > Since all color operations exposed by KMS are standard, the library
> > > > would just be a simple one: no loader, no plugin, no proprietary pieces,
> > > > etc.
> > > >
> > >
> > > There might be pipelines/color-ops only exposed by proprietary out of
> > > tree drivers; the operation types and semantics should ideally be
> > > defined upstream, but the code paths would in practice be vendor
> > > specific, potentially without any upstream driver using them. It should
> > > be clear whether an implementation that makes such a pipeline work is in
> > > scope for the upstream library.
> > >
> > > The same applies to the kernel; it must be clear whether pipeline
> > > elements that potentially will only be exposed by out of tree drivers
> > > will be acceptable upstream, at least as documented operations.
> > >
> >
>

Re: [RFC] Plane color pipeline KMS uAPI

2023-05-11 Thread Simon Ser
On Thursday, May 11th, 2023 at 18:56, Joshua Ashton  wrote:

> When we are talking about being 'prescriptive' in the API, are we
> outright saying we don't want to support arbitrary 3D LUTs, or are we
> just offering certain algorithms to be 'executed' for a plane/crtc/etc
> in the atomic API? I am confused...

>From a kernel PoV:

- Prescriptive = here are the available hardware blocks, feel free to
  configure each as you like
- Descriptive = give me the source and destination color-spaces and I
  take care of everything

This proposal is a prescriptive API. We haven't explored _that_ much
how a descriptive API would look like, probably it can include some way
to do Night Light and similar features but not sure how high-level
they'd look like. A descriptive API is inherently more restrictive than
a prescriptive API.


Re: [RFC] Plane color pipeline KMS uAPI

2023-05-11 Thread Simon Ser
On Friday, May 5th, 2023 at 15:30, Joshua Ashton  wrote:

> > > AMD would expose the following objects and properties:
> > >
> > > Plane 10
> > > β”œβ”€ "type": immutable enum {Overlay, Primary, Cursor} = Primary
> > > └─ "color_pipeline": enum {0, 42} = 0
> > > Color operation 42 (input CSC)
> > > β”œβ”€ "type": enum {Bypass, Matrix} = Matrix
> > > β”œβ”€ "matrix_data": blob
> > > └─ "next": immutable color operation ID = 43
> > > Color operation 43
> > > β”œβ”€ "type": enum {Scaling} = Scaling
> > > └─ "next": immutable color operation ID = 44
> > > Color operation 44 (DeGamma)
> > > β”œβ”€ "type": enum {Bypass, 1D curve} = 1D curve
> > > β”œβ”€ "1d_curve_type": enum {sRGB, PQ, …} = sRGB
> > > └─ "next": immutable color operation ID = 45
> 
> Some vendors have per-tap degamma and some have a degamma after the sample.
> How do we distinguish that behaviour?
> It is important to know.

Can you elaborate? What is "per-tap" and "sample"? Is the "Scaling" color
operation above not enough to indicate where in the pipeline the hw performs
scaling?

> > > Color operation 45 (gamut remap)
> > > β”œβ”€ "type": enum {Bypass, Matrix} = Matrix
> > > β”œβ”€ "matrix_data": blob
> > > └─ "next": immutable color operation ID = 46
> > > Color operation 46 (shaper LUT RAM)
> > > β”œβ”€ "type": enum {Bypass, 1D curve} = 1D curve
> > > β”œβ”€ "1d_curve_type": enum {LUT} = LUT
> > > β”œβ”€ "lut_size": immutable range = 4096
> > > β”œβ”€ "lut_data": blob
> > > └─ "next": immutable color operation ID = 47
> > > Color operation 47 (3D LUT RAM)
> > > β”œβ”€ "type": enum {Bypass, 3D LUT} = 3D LUT
> > > β”œβ”€ "lut_size": immutable range = 17
> > > β”œβ”€ "lut_data": blob
> > > └─ "next": immutable color operation ID = 48
> > > Color operation 48 (blend gamma)
> > > β”œβ”€ "type": enum {Bypass, 1D curve} = 1D curve
> > > β”œβ”€ "1d_curve_type": enum {LUT, sRGB, PQ, …} = LUT
> > > β”œβ”€ "lut_size": immutable range = 4096
> > > β”œβ”€ "lut_data": blob
> > > └─ "next": immutable color operation ID = 0
> > >
> > > To configure the pipeline for an HDR10 PQ plane (path at the top) and a 
> > > HDR
> > > display, gamescope would perform an atomic commit with the following 
> > > property
> > > values:
> > >
> > > Plane 10
> > > └─ "color_pipeline" = 42
> > > Color operation 42 (input CSC)
> > > └─ "matrix_data" = PQ β†’ scRGB (TF)
> 
> ^
> Not sure what this is.
> We don't use an input CSC before degamma.
> 
> > > Color operation 44 (DeGamma)
> > > └─ "type" = Bypass
> 
> ^
> If we did PQ, this would be PQ -> Linear / 80
> If this was sRGB, it'd be sRGB -> Linear
> If this was scRGB this would be just treating it as it is. So... Linear / 80.
> 
> > > Color operation 45 (gamut remap)
> > > └─ "matrix_data" = scRGB (TF) β†’ PQ
> 
> ^
> This is wrong, we just use this to do scRGB primaries (709) to 2020.
> 
> We then go from scRGB -> PQ to go into our shaper + 3D LUT.
> 
> > > Color operation 46 (shaper LUT RAM)
> > > └─ "lut_data" = PQ β†’ Display native
> 
> ^
> "Display native" is just the response curve of the display.
> In HDR10, this would just be PQ -> PQ
> If we were doing HDR10 on SDR, this would be PQ -> Gamma 2.2 (mapped
> from 0 to display native luminance) [with a potential bit of headroom
> for tonemapping in the 3D LUT]
> For SDR on HDR10 this would be Gamma 2.2 -> PQ (Not intending to start
> an sRGB vs G2.2 argument here! :P)
> 
> > > Color operation 47 (3D LUT RAM)
> > > └─ "lut_data" = Gamut mapping + tone mapping + night mode
> > > Color operation 48 (blend gamma)
> > > └─ "1d_curve_type" = PQ
> 
> ^
> This is wrong, this should be Display Native -> Linearized Display Referred

In the HDR case, isn't this the inverse of PQ?

> > You cannot do a TF with a matrix, and a gamut remap with a matrix on
> > electrical values is certainly surprising, so the example here is a
> > bit odd, but I don't think that hurts the intention of demonstration.
> 
> I have done some corrections inline.
> 
> You can see our fully correct color pipeline here:
> https://raw.githubusercontent.com/ValveSoftware/gamescope/master/src/docs/Steam%20Deck%20Display%20Pipeline.png
> 
> Please let me know if you have any more questions about our color pipeline.

As expected, I got the gamescope part wrong. I'm pretty confident that the
proposed API would still work since the AMD vendor-specific props would just
be exposed as color operation objects. Can you confirm we can make the
gamescope pipeline work with the AMD color pipeline outlined above?