Re: [RFC] Plane color pipeline KMS uAPI

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 9645 bytes --]

On Fri, 5 May 2023 21:51:41 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, May 05, 2023 at 05:57:37PM +0200, Sebastian Wick wrote:
> > On Fri, May 5, 2023 at 5:28 PM Daniel Vetter <daniel@ffwll.ch> wrote:  
> > >
> > > On Thu, May 04, 2023 at 03:22:59PM +0000, Simon Ser wrote:  
> > > > Hi all,
> > > >
> > > > The goal of this RFC is to expose a generic KMS uAPI to configure the color
> > > > pipeline before blending, ie. after a pixel is tapped from a plane's
> > > > framebuffer and before it's blended with other planes. With this new uAPI we
> > > > aim to reduce the battery life impact of color management and HDR on mobile
> > > > devices, to improve performance and to decrease latency by skipping
> > > > composition on the 3D engine. This proposal is the result of discussions at
> > > > the Red Hat HDR hackfest [1] which took place a few days ago. Engineers
> > > > familiar with the AMD, Intel and NVIDIA hardware have participated in the
> > > > discussion.
> > > >
> > > > This proposal takes a prescriptive approach instead of a descriptive approach.
> > > > Drivers describe the available hardware blocks in terms of low-level
> > > > mathematical operations, then user-space configures each block. We decided
> > > > against a descriptive approach where user-space would provide a high-level
> > > > description of the colorspace and other parameters: we want to give more
> > > > control and flexibility to user-space, e.g. to be able to replicate exactly the
> > > > color pipeline with shaders and switch between shaders and KMS pipelines
> > > > seamlessly, and to avoid forcing user-space into a particular color management
> > > > policy.  
> > >
> > > Ack on the prescriptive approach, but generic imo. Descriptive pretty much
> > > means you need the shaders at the same api level for fallback purposes,
> > > and we're not going to have that ever in kms. That would need something
> > > like hwc in userspace to work.  
> > 
> > Which would be nice to have but that would be forcing a specific color
> > pipeline on everyone and we explicitly want to avoid that. There are
> > just too many trade-offs to consider.
> >   
> > > And not generic in it's ultimate consquence would mean we just do a blob
> > > for a crtc with all the vendor register stuff like adf (android display
> > > framework) does, because I really don't see a point in trying a
> > > generic-looking-but-not vendor uapi with each color op/stage split out.
> > >
> > > So from very far and pure gut feeling, this seems like a good middle
> > > ground in the uapi design space we have here.  
> > 
> > Good to hear!
> >   
> > > > We've decided against mirroring the existing CRTC properties
> > > > DEGAMMA_LUT/CTM/GAMMA_LUT onto KMS planes. Indeed, the color management
> > > > pipeline can significantly differ between vendors and this approach cannot
> > > > accurately abstract all hardware. In particular, the availability, ordering and
> > > > capabilities of hardware blocks is different on each display engine. So, we've
> > > > decided to go for a highly detailed hardware capability discovery.
> > > >
> > > > This new uAPI should not be in conflict with existing standard KMS properties,
> > > > since there are none which control the pre-blending color pipeline at the
> > > > moment. It does conflict with any vendor-specific properties like
> > > > NV_INPUT_COLORSPACE or the patches on the mailing list adding AMD-specific
> > > > properties. Drivers will need to either reject atomic commits configuring both
> > > > uAPIs, or alternatively we could add a DRM client cap which hides the vendor
> > > > properties and shows the new generic properties when enabled.
> > > >
> > > > To use this uAPI, first user-space needs to discover hardware capabilities via
> > > > KMS objects and properties, then user-space can configure the hardware via an
> > > > atomic commit. This works similarly to the existing KMS uAPI, e.g. planes.
> > > >
> > > > Our proposal introduces a new "color_pipeline" plane property, and a new KMS
> > > > object type, "COLOROP" (short for color operation). The "color_pipeline" plane
> > > > property is an enum, each enum entry represents a color pipeline supported by
> > > > the hardware. The special zero entry indicates that the pipeline is in
> > > > "bypass"/"no-op" mode. For instance, the following plane properties describe a
> > > > primary plane with 2 supported pipelines but currently configured in bypass
> > > > mode:
> > > >
> > > >     Plane 10
> > > >     ├─ "type": immutable enum {Overlay, Primary, Cursor} = Primary
> > > >     ├─ …
> > > >     └─ "color_pipeline": enum {0, 42, 52} = 0  
> > >
> > > A bit confused, why is this an enum, and not just an immutable prop that
> > > points at the first element? You already can disable elements with the
> > > bypass thing, also bypassing by changing the pointers to the next node in
> > > the graph seems a bit confusing and redundant.  
> > 
> > We want to allow multiple pipelines to exist and a plane can choose
> > the pipeline by selecting the first element of the pipeline. The enum
> > here lists all the possible pipelines that can be attached to the
> > surface.  
> 
> Ah in that case I guess we do need the flexibility of explicitly
> enumerated object property right away I guess. The example looked a bit
> like just bypass would do the trick.

Setting individual pipeline elements to bypass is not flexible enough,
because it does not allow re-ordering the pipeline elements.

OTOH, hardware does not allow arbitrary re-ordering of elements,
therefore all the "next" links in the pipeline elements are immutable.

Presumably when some re-ordering in hardware is possible, the number of
order-variants is small (given we can also bypass individual elements),
so all variants are explicitly enumerated at the plane property.

This way there are no traps like "you cannot enable all elements at the
same time" that a single standard pipeline would end up with. All
elements for all enumerated pipelines are always usable simultaneously,
I believe.

"Re-ordering" may not even be an accurate description of hardware. Some
hardware elements could be mutually exclusive, whether they implement
the same or a different operation.

> > > > The non-zero entries describe color pipelines as a linked list of COLOROP KMS
> > > > objects. The entry value is an object ID pointing to the head of the linked
> > > > list (the first operation in the color pipeline).
> > > >
> > > > The new COLOROP objects also expose a number of KMS properties. Each has a
> > > > type, a reference to the next COLOROP object in the linked list, and other
> > > > type-specific properties. Here is an example for a 1D LUT operation:  
> > >
> > > Ok no comments from me on the actual color operations and semantics of all
> > > that, because I have simply nothing to bring to that except confusion :-)
> > >
> > > Some higher level thoughts instead:
> > >
> > > - I really like that we just go with graph nodes here. I think that was
> > >   bound to happen sooner or later with kms (we almost got there with
> > >   writeback, and with hindsight maybe should have).
> > >
> > > - Since there's other use-cases for graph nodes (maybe scaler modes, or
> > >   histogram samplers for adaptive backglight, or blending that goes beyond
> > >   the stacked alpha blending we have now) it think we should make this all
> > >   fairly generic:
> > >   * Add a new graph node kms object type.
> > >   * Add a class type so that userspace knows which graph nodes it must
> > >     understand for a feature (like "ColorOp" on planes here), and which it
> > >     can ignore (like perhaps a scaler node to control the interpolation)
> > >   * Probably need to adjust the object property type. Currently that
> > >     accept any object of a given type (crtc, fb, blob are the major ones).
> > >     I think for these graph nodes we want an explicit enumeration of the
> > >     possible next objects. In kms thus far we've done that with the
> > >     separate possible_* mask properties, but they're cumbersome.
> > >   * It sounds like for now we only have immutable next pointers, so that
> > >     would simplify the first iteration, but should probably anticipate all
> > >     this.  
> > 
> > Just to be clear: right now we don't expect any pipeline to be a graph
> > but only linked lists. It probably doesn't hurt to generalize this to
> > graphs but that's not what we want to do here (for now).  
> 
> Oh a list is still a graph :-) Also my idea isn't to model a graph data
> structure, but just the graph nodes, and a bit of scaffolding to handle
> the links/pointers. Whether you only build a list of a graph out of that
> is kinda irrelevant.
> 
> Plus with the multiple pipelines you can already have a non-list in the
> starting point already.

No, there is no need for a graph. It literally is just multiple
single-linked lists at the UAPI level, and userspace picks one for the
plane.

If you change the immutable "next element" properties to mutable, then
I think the UAPI collapses. It becomes far too hard to know what even
could work, and probing for success/failure like with KMS in general is
simply infeasible.

Let's leave the more wild scenarios for the second total rewrite of the
KMS color API (assuming this is the start of the first). Otherwise we
get nowhere. That's what Wayland protocol design work has taught us.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]