On Friday, May 5th, 2023 at 21:53, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Fri, May 05, 2023 at 04:06:26PM +0000, Simon Ser wrote:
> > On Friday, May 5th, 2023 at 17:28, Daniel Vetter <daniel@xxxxxxxx> 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.
