Re: [RFC] Plane color pipeline KMS uAPI

2023-05-08 Thread Pekka Paalanen
On Fri, 5 May 2023 21:51:41 +0200
Daniel Vetter  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  wrote:  
> > >
> > > On Thu, May 04, 2023 at 03:22:59PM +, 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 di

Re: [RFC] Plane color pipeline KMS uAPI

2023-05-08 Thread Pekka Paalanen
On Fri, 5 May 2023 16:04:35 -0500
Steven Kucharzyk  wrote:

> Hi,
> 
> I'm new to this list and probably can't contribute but interested, I
> passed your original posting to a friend and have enclosed his thoughts
> ... old hash or food for thought ??? I ask your forgiveness if you find
> this inappropriate. (am of the elk * act first, ask for forgiveness
> afterward)  ;-)

Thanks, but please use reply-to-all, it's a bit painful to add back all
the other mailing lists and people.

> 
> Steven
> 
> - start 
> 
> Steven Kucharzyk wrote:
> 
> > Thought you might find this of interest.
> 
> Hi,
>   thanks for sending it to me.
> 
> Unfortunately I don't know enough about the context to say anything
> specific about it.
> 
> The best I can do is state the big picture aims I would
> look for, as someone with a background in display systems
> electronic design, rendering software development
> and Color Science. (I apologize in advance if any of this
> is preaching to the choir!)
> 
> 1) I would make sure that someone with a strong Color Science
> background was consulted in the development of the API.

Where can we find someone like that, who would also not start by saying
we cannot get anything right, or that we cannot change the old software
architecture, and would actually try to understand *our* goals and
limitations as well? Who could commit to long discussions over several
years in a *friendly* manner?

It would take extreme amounts of patience from that person.

> 2) I would be measuring the API against its ability to
> support a "profiling" color management workflow. This workflow
> allows using the full capability of a display, while also allowing
> simultaneous display of multiple sources encoded in any colorspace.
> So the basic architecture is to have a final frame buffer (real
> or virtual) in the native displays colorspace, and use any
> graphics hardware color transform and rendering capability to
> assist with the transformation of data in different source
> colorspaces into the displays native colorspace.
> 
> 3) The third thing I would be looking for, is enough
> standardization that user mode software can be written
> that will get key benefits of what's available in the hardware,
> without needing to be customized to lots of different hardware
> specifics. For instance, I'd make sure that there was a standard display
> frame buffer to display mode that applied per channel curves
> that are specified in a standard way. (i.e. make sure that there
> is an easy to use replacement for XRRCrtcGamma.)
> 
> Any API that is specific to a type or model of graphics card,
> will retard development of color management support to a very large
> degree - the financial and development costs of obtaining, configuring
> and testing against multiple graphic card makes and models puts this
> in the too hard basket for anyone other than a corporation.
> 
> Perhaps little of the above is relevant, if this is a low level API
> that is to be used by other operating system sub-systems such
> as display graphics API's like X11 or Wayland, which will choose
> specific display rendering models and implement them with the hardware
> capabilities that are available.

That is exactly what it is. It is a way to save power and gain
performance when things happen to fit in place just right: what one
needs to do matches what the dedicated color processing hardware blocks
implement.

> From a color management point of view,
> it is the operating system & UI graphics API's that are the ones that
> are desirable to work with, since they are meant to insulate
> applications from hardware details.

Indeed. Anything the display controller hardware cannot do will be
implemented by other means, e.g. on the GPU, by a display server.


Thanks,
pq


pgp_2QLCJF6HZ.pgp
Description: OpenPGP digital signature


Re: [RFC] Plane color pipeline KMS uAPI

2023-05-08 Thread Simon Ser
On Friday, May 5th, 2023 at 21:53, Daniel Vetter  wrote:

> On Fri, May 05, 2023 at 04:06:26PM +, Simon Ser wrote:
> > On Friday, May 5th, 2023 at 17:28, Daniel Vetter  wrote:
> >
> > > 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).
> >
> > I'd really rather not do graphs here. We only need linked lists as Sebastian
> > said. Graphs would significantly add more complexity to this proposal, and
> > I don't think that's a good idea unless there is a strong use-case.
> 
> You have a graph, because a graph is just nodes + links. I did _not_
> propose a full generic graph structure, the link pointer would be in the
> class/type specific structure only. Like how we have the plane->crtc or
> connector->crtc links already like that (which already _is_ is full blown
> graph).

I really don't get why a pointer in a struct makes plane->crtc a full-blown
graph. There is only a single parent-child link. A plane has a reference to a
CRTC, and nothing more.

You could say that anything is a graph. Yes, even an isolated struct somewhere
is a graph: one with a single node and no link. But I don't follow what's the
point of explaining everything with a graph when we only need a much simpler
subset of the concept of graphs?

Putting the graph thing aside, what are you suggesting exactly from a concrete
uAPI point-of-view? Introducing a new struct type? Would it be a colorop
specific struct, or a more generic one? What would be the fields? Why do you
think that's necessary and better than the current proposal?

My understanding so far is that you're suggesting introducing something like
this at the uAPI level:

struct drm_mode_node {
uint32_t id;

uint32_t children_count;
uint32_t *children; // list of child object IDs
};

I don't think this is a good idea for multiple reasons. First, this is
overkill: we don't need this complexity, and this complexity will make it more
difficult to reason about the color pipeline. This is a premature abstraction,
one we don't need right now, and one I heaven't heard a potential future
use-case for. Sure, one can kill an ant with a sledgehammer if they'd like, but
that's not the right tool for the job.

Second, this will make user-space miserable. User-space already has a tricky
task to achieve to translate its abstract descriptive color pipeline to our
proposed simple list of color operations. If we expose a full-blown graph, then
the user-space logic will need to handle arbitrary graphs. This will have a
significant cost (on implementation and testing), which we will be paying in
terms of time spent and in terms of bugs.

Last, this kind of generic "node" struct is at odds with existing KMS object
types. So far, KMS objects are concrete like CRTC, connector, plane, etc.
"Node" is abstract. This is inconsistent.

Please let me know whether the above is what you have in mind. If not, please
explain what exactly you mean by "graphs" in terms of uAPI, and please explain
why we need it and what real-world use-cases it would solve.


Re: [RFC] Plane color pipeline KMS uAPI

2023-05-08 Thread Daniel Vetter
On Mon, 8 May 2023 at 10:24, Pekka Paalanen  wrote:
>
> On Fri, 5 May 2023 21:51:41 +0200
> Daniel Vetter  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  wrote:
> > > >
> > > > On Thu, May 04, 2023 at 03:22:59PM +, 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:
> > > > >
> > > > > Pla

Re: [RFC] Plane color pipeline KMS uAPI

2023-05-08 Thread Daniel Vetter
On Mon, 8 May 2023 at 10:58, Simon Ser  wrote:
>
> On Friday, May 5th, 2023 at 21:53, Daniel Vetter  wrote:
>
> > On Fri, May 05, 2023 at 04:06:26PM +, Simon Ser wrote:
> > > On Friday, May 5th, 2023 at 17:28, Daniel Vetter  wrote:
> > >
> > > > 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).
> > >
> > > I'd really rather not do graphs here. We only need linked lists as 
> > > Sebastian
> > > said. Graphs would significantly add more complexity to this proposal, and
> > > I don't think that's a good idea unless there is a strong use-case.
> >
> > You have a graph, because a graph is just nodes + links. I did _not_
> > propose a full generic graph structure, the link pointer would be in the
> > class/type specific structure only. Like how we have the plane->crtc or
> > connector->crtc links already like that (which already _is_ is full blown
> > graph).
>
> I really don't get why a pointer in a struct makes plane->crtc a full-blown
> graph. There is only a single parent-child link. A plane has a reference to a
> CRTC, and nothing more.
>
> You could say that anything is a graph. Yes, even an isolated struct somewhere
> is a graph: one with a single node and no link. But I don't follow what's the
> point of explaining everything with a graph when we only need a much simpler
> subset of the concept of graphs?
>
> Putting the graph thing aside, what are you suggesting exactly from a concrete
> uAPI point-of-view? Introducing a new struct type? Would it be a colorop
> specific struct, or a more generic one? What would be the fields? Why do you
> think that's necessary and better than the current proposal?
>
> My understanding so far is that you're suggesting introducing something like
> this at the uAPI level:
>
> struct drm_mode_node {
> uint32_t id;
>
> uint32_t children_count;
> uint32_t *children; // list of child object IDs
> };

Already too much I think

struct drm_mode_node {
struct drm_mode_object base;
struct drm_private_obj atomic_base;
enum drm_mode_node_enum type;
};

The actual graph links would be in the specific type's state
structure, like they are for everything else. And the limits would be
on the property type, we probably need a new DRM_MODE_PROP_OBJECT_ENUM
to make the new limitations work correctly, since the current
DRM_MODE_PROP_OBJECT only limits to a specific type of object, not an
explicit list of drm_mode_object.id.

You might not even need a node subclass for the state stuff, that
would directly be a drm_color_op_state that only embeds
drm_private_state.

Another uapi difference is that the new kms objects would be of type
DRM_MODE_OBJECT_NODE, and would always have a "class" property.

> I don't think this is a good idea for multiple reasons. First, this is
> overkill: we don't need this complexity, and this complexity will make it more
> difficult to reason about the color pipeline. This is a premature abstraction,
> one we don't need right now, and one I heaven't heard a potential future
> use-case for. Sure, one can kill an ant with a sledgehammer if they'd like, 
> but
> that's not the right tool for the job.
>
> Second, this will make user-space miserable. User-space already has a tricky
> task to achieve to translate its abstract descriptive color pipeline to our
> proposed simple list of color operations. If we expose a full-blown graph, 
> then
> the user-space logic will need to handle arbitrary graphs. This will have a
> significant cost (on implementation and testing), which we will be paying in
> terms of time spent and in terms of bugs.

The color op pipeline would still be linear. I did not ask for a non-linear one.

> Last, this kind of generic "node" struct is at odds with existing KMS object
> types. So far, KMS objects are concrete like CRTC, connector, plane, etc.
> "Node" is abstract. This is inconsistent.

Yeah I think I think we should change that. That's essentially the
full extend of my proposal. The classes + possible_foo mask approach
just always felt rather brittle to me (and there's plenty of userspace
out there to prove that's the case), going more explicit with the
links with enumerated combos feels better. Plus it should allow
building a bit cleaner interfaces for drivers to construct the correct
graphs, because drivers _also_ rather consistently got the entire
possible_foo mask business wrong.

> Please let me know whether the above is what you have in mind. If not, please
> explain what exactly you mean by "graphs" in terms of uAPI, and please explain
> why we need it and what real-world use-cases it would

Re: [RFC] Plane color pipeline KMS uAPI

2023-05-08 Thread Pekka Paalanen
On Mon, 8 May 2023 09:14:18 +1000
Dave Airlie  wrote:

> On Sat, 6 May 2023 at 08:21, Sebastian Wick  wrote:
> >
> > On Fri, May 5, 2023 at 10:40 PM Dave Airlie  wrote:  
> > >
> > > On Fri, 5 May 2023 at 01:23, 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.  
> > >
> > > I'm not 100% sold on the prescriptive here, let's see if someone can
> > > get me over the line with some questions later.

Hi Dave,

generic userspace must always be able to fall back to GPU shaders or
something else, when a window suddenly stops being eligible for a KMS
plane. That can happen due to a simple window re-stacking operation for
example, maybe a notification pops up temporarily. Hence, it is highly
desirable to be able to implement the exact same algorithm in shaders
as the display hardware does, in order to not cause visible glitches
on screen.

One way to do that is to have a prescriptive UAPI design. Userspace
decides what algorithms to use for color processing, and the UAPI simply
offers a way to implement those well-defined mathematical operations.
An alternative could be that the UAPI gives userspace back shader
programs that implement the same as what the hardware does, but... ugh.

Choosing the algorithm is policy. Userspace must be in control of
policy, right? Therefore a descriptive UAPI is simply not possible.
There is no single correct algorithm for these things, there are many
flavors, more and less correct, different quality/performance
trade-offs, and even just matters of taste. Sometimes even end user
taste, that might need to be configurable. Applications have built-in
assumptions too, and they vary.

To clarify, a descriptive UAPI is a design where userspace tells the
kernel "my source 1 is sRGB, my source 2 is BT.2100/PQ YCbCr 4:2:0 with
blahblahblah metadata, do whatever to display those on KMS planes
simultaneously". As I mentioned, there is not just one answer to that,
and we should also allow for innovation in the algorithms by everyone,
not just hardware designers.

A prescriptive UAPI is where we communicate mathematical operations
without any semantics. It is inherently free of policy in the kernel.

> > >
> > > My feeling is color pipeline hw is not a done deal, and that hw
> > > vendors will be revising/evolving/churning the hw blocks for a while
> > > longer, as there is no real standards in the area to aim for, all the
> > > vendors are mostly just doing whatever gets Windows over the line and
> > > keeps hw engineers happy. So I have some concerns here around forwards
> > > compatibility and hence the API design.
> > >
> > > I guess my main concern is if you expose a bunch of hw blocks and
> > > someone comes up with a novel new thing, will all existing userspace
> > > work, without falling back to shaders?
> > > Do we have minimum guarantees on what hardware blocks have to be
> > > exposed to build a useable pipeline?
> > > If a hardware block goes away in a new silicon revision, do I have to
> > > rewrite my compositor? or will it be expected that the kernel will
> > > emulate the old pipelines on top of whatever new fancy thing exists.  
> >
> > I think there are two answers to those questions.  
> 
> These aren't selling me much better :-)
> >
> > The first one is that right now KMS already doesn't guarantee that
> > every property is supported on all hardware. The guarantee we have is
> > that properties that are supported on a piece of hardware on a
> > specific kernel will be supported on the same hardware on later
> > kernels. The color pipeline is no diffe

Re: [RFC] Plane color pipeline KMS uAPI

2023-05-08 Thread Jonas Ådahl
On Mon, May 08, 2023 at 09:14:18AM +1000, Dave Airlie wrote:
> On Sat, 6 May 2023 at 08:21, Sebastian Wick  wrote:
> >
> > On Fri, May 5, 2023 at 10:40 PM Dave Airlie  wrote:
> > >
> > > On Fri, 5 May 2023 at 01:23, 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.
> > >
> > > I'm not 100% sold on the prescriptive here, let's see if someone can
> > > get me over the line with some questions later.
> > >
> > > My feeling is color pipeline hw is not a done deal, and that hw
> > > vendors will be revising/evolving/churning the hw blocks for a while
> > > longer, as there is no real standards in the area to aim for, all the
> > > vendors are mostly just doing whatever gets Windows over the line and
> > > keeps hw engineers happy. So I have some concerns here around forwards
> > > compatibility and hence the API design.
> > >
> > > I guess my main concern is if you expose a bunch of hw blocks and
> > > someone comes up with a novel new thing, will all existing userspace
> > > work, without falling back to shaders?
> > > Do we have minimum guarantees on what hardware blocks have to be
> > > exposed to build a useable pipeline?
> > > If a hardware block goes away in a new silicon revision, do I have to
> > > rewrite my compositor? or will it be expected that the kernel will
> > > emulate the old pipelines on top of whatever new fancy thing exists.
> >
> > I think there are two answers to those questions.
> 
> These aren't selling me much better :-)
> >
> > The first one is that right now KMS already doesn't guarantee that
> > every property is supported on all hardware. The guarantee we have is
> > that properties that are supported on a piece of hardware on a
> > specific kernel will be supported on the same hardware on later
> > kernels. The color pipeline is no different here. For a specific piece
> > of hardware a newer kernel might only change the pipelines in a
> > backwards compatible way and add new pipelines.
> >
> > So to answer your question: if some hardware with a novel pipeline
> > will show up it might not be supported and that's fine. We already
> > have cases where some hardware does not support the gamma lut property
> > but only the CSC property and that breaks night light because we never
> > bothered to write a shader fallback. KMS provides ways to offload work
> > but a generic user space always has to provide a fallback and this
> > doesn't change. Hardware specific user space on the other hand will
> > keep working with the forward compatibility guarantees we want to
> > provide.
> 
> In my mind we've screwed up already, isn't a case to be made for
> continue down the same path.
> 
> The kernel is meant to be a hardware abstraction layer, not just a
> hardware exposure layer. The kernel shouldn't set policy and there are
> cases where it can't act as an abstraction layer (like where you need
> a compiler), but I'm not sold that this case is one of those yet. I'm
> open to being educated here on why it would be.

It would still be an abstraction of the hardware, just that the level
of abstraction is a bit "lower" than your intuition currently tells you
we should have. IMO it's not too different from the kernel providing low
level input events describing what what the hardware can do and does,
with a rather massive user space library (libinput) turning all of that
low level nonsense to actual useful abstractions.

In this case it's the other way around, the kernel provides vendor
independent knobs that describe what the output hardware can do, and
exactly how it does

Re: [RFC] Plane color pipeline KMS uAPI

2023-05-08 Thread Harry Wentland



On 5/8/23 05:18, Daniel Vetter wrote:
> On Mon, 8 May 2023 at 10:58, Simon Ser  wrote:
>>
>> On Friday, May 5th, 2023 at 21:53, Daniel Vetter  wrote:
>>
>>> On Fri, May 05, 2023 at 04:06:26PM +, Simon Ser wrote:
 On Friday, May 5th, 2023 at 17:28, Daniel Vetter  wrote:

> 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).

 I'd really rather not do graphs here. We only need linked lists as 
 Sebastian
 said. Graphs would significantly add more complexity to this proposal, and
 I don't think that's a good idea unless there is a strong use-case.
>>>
>>> You have a graph, because a graph is just nodes + links. I did _not_
>>> propose a full generic graph structure, the link pointer would be in the
>>> class/type specific structure only. Like how we have the plane->crtc or
>>> connector->crtc links already like that (which already _is_ is full blown
>>> graph).
>>
>> I really don't get why a pointer in a struct makes plane->crtc a full-blown
>> graph. There is only a single parent-child link. A plane has a reference to a
>> CRTC, and nothing more.
>>
>> You could say that anything is a graph. Yes, even an isolated struct 
>> somewhere
>> is a graph: one with a single node and no link. But I don't follow what's the
>> point of explaining everything with a graph when we only need a much simpler
>> subset of the concept of graphs?
>>
>> Putting the graph thing aside, what are you suggesting exactly from a 
>> concrete
>> uAPI point-of-view? Introducing a new struct type? Would it be a colorop
>> specific struct, or a more generic one? What would be the fields? Why do you
>> think that's necessary and better than the current proposal?
>>
>> My understanding so far is that you're suggesting introducing something like
>> this at the uAPI level:
>>
>> struct drm_mode_node {
>> uint32_t id;
>>
>> uint32_t children_count;
>> uint32_t *children; // list of child object IDs
>> };
> 
> Already too much I think
> 
> struct drm_mode_node {
> struct drm_mode_object base;
> struct drm_private_obj atomic_base;
> enum drm_mode_node_enum type;
> };
> 

This would be about as much as we would want for a 'node' struct, for
reasons that others already outlined. In short, a good API for a color
pipeline needs to do a good job communicating the constraints. Hence the
"next" pointer needs to be live in a colorop struct, whether it's a
drm_private_obj or its own thing.

I'm not quite seeing much benefits with a drm_mode_node other than being
able to have a GET_NODE IOCTL instead of a GET_COLOROP, the former being
able to be re-used for future scenarios that might need a "node." I feel
this adds a layer of confusion to the API.

Harry

> The actual graph links would be in the specific type's state
> structure, like they are for everything else. And the limits would be
> on the property type, we probably need a new DRM_MODE_PROP_OBJECT_ENUM
> to make the new limitations work correctly, since the current
> DRM_MODE_PROP_OBJECT only limits to a specific type of object, not an
> explicit list of drm_mode_object.id.
> 
> You might not even need a node subclass for the state stuff, that
> would directly be a drm_color_op_state that only embeds
> drm_private_state.
> 
> Another uapi difference is that the new kms objects would be of type
> DRM_MODE_OBJECT_NODE, and would always have a "class" property.
> 
>> I don't think this is a good idea for multiple reasons. First, this is
>> overkill: we don't need this complexity, and this complexity will make it 
>> more
>> difficult to reason about the color pipeline. This is a premature 
>> abstraction,
>> one we don't need right now, and one I heaven't heard a potential future
>> use-case for. Sure, one can kill an ant with a sledgehammer if they'd like, 
>> but
>> that's not the right tool for the job.
>>
>> Second, this will make user-space miserable. User-space already has a tricky
>> task to achieve to translate its abstract descriptive color pipeline to our
>> proposed simple list of color operations. If we expose a full-blown graph, 
>> then
>> the user-space logic will need to handle arbitrary graphs. This will have a
>> significant cost (on implementation and testing), which we will be paying in
>> terms of time spent and in terms of bugs.
> 
> The color op pipeline would still be linear. I did not ask for a non-linear 
> one.
> 
>> Last, this kind of generic "node" struct is at odds with existing KMS object
>> types. So far, KMS objects are concrete like CRTC, connector, plane, etc.
>> "Node" is abstract. This is inconsistent.
> 
> Yeah I think I 

Re: [RFC] Plane color pipeline KMS uAPI

2023-05-08 Thread Steven Kucharzyk
On Mon, 08 May 2023 08:58:37 +
Simon Ser  wrote:

> On Friday, May 5th, 2023 at 21:53, Daniel Vetter 
> wrote:
> 
> > On Fri, May 05, 2023 at 04:06:26PM +, Simon Ser wrote:
> > > On Friday, May 5th, 2023 at 17:28, Daniel Vetter
> > >  wrote:
> > >
> > > > 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).
> > >
> > > I'd really rather not do graphs here. We only need linked lists
> > > as Sebastian said. Graphs would significantly add more complexity
> > > to this proposal, and I don't think that's a good idea unless
> > > there is a strong use-case.
> > 
> > You have a graph, because a graph is just nodes + links. I did _not_
> > propose a full generic graph structure, the link pointer would be
> > in the class/type specific structure only. Like how we have the
> > plane->crtc or connector->crtc links already like that (which
> > already _is_ is full blown graph).
> 
> I really don't get why a pointer in a struct makes plane->crtc a
> full-blown graph. There is only a single parent-child link. A plane
> has a reference to a CRTC, and nothing more.
> 
> You could say that anything is a graph. Yes, even an isolated struct
> somewhere is a graph: one with a single node and no link. But I don't
> follow what's the point of explaining everything with a graph when we
> only need a much simpler subset of the concept of graphs?
> 
> Putting the graph thing aside, what are you suggesting exactly from a
> concrete uAPI point-of-view? Introducing a new struct type? Would it
> be a colorop specific struct, or a more generic one? What would be
> the fields? Why do you think that's necessary and better than the
> current proposal?
> 
> My understanding so far is that you're suggesting introducing
> something like this at the uAPI level:
> 
> struct drm_mode_node {
> uint32_t id;
> 
> uint32_t children_count;
> uint32_t *children; // list of child object IDs
> };
> 
> I don't think this is a good idea for multiple reasons. First, this is
> overkill: we don't need this complexity, and this complexity will
> make it more difficult to reason about the color pipeline. This is a
> premature abstraction, one we don't need right now, and one I
> heaven't heard a potential future use-case for. Sure, one can kill an
> ant with a sledgehammer if they'd like, but that's not the right tool
> for the job.
> 
> Second, this will make user-space miserable. User-space already has a
> tricky task to achieve to translate its abstract descriptive color
> pipeline to our proposed simple list of color operations. If we
> expose a full-blown graph, then the user-space logic will need to
> handle arbitrary graphs. This will have a significant cost (on
> implementation and testing), which we will be paying in terms of time
> spent and in terms of bugs.
> 
> Last, this kind of generic "node" struct is at odds with existing KMS
> object types. So far, KMS objects are concrete like CRTC, connector,
> plane, etc. "Node" is abstract. This is inconsistent.
> 
> Please let me know whether the above is what you have in mind. If
> not, please explain what exactly you mean by "graphs" in terms of
> uAPI, and please explain why we need it and what real-world use-cases
> it would solve.

I'd like to ask if there is a block/flow chart/diagram that has been
created that represent the elements that are being discussed for this
RFC? If so, would you be so kind as to point me to it or send it to me?

Tks, Steven