From: Harry Wentland <harry.wentland@amd.com>
To: Daniel Vetter <daniel@ffwll.ch>, Simon Ser <contact@emersion.fr>
Cc: "Pekka Paalanen" <pekka.paalanen@collabora.com>,
wayland-devel <wayland-devel@lists.freedesktop.org>,
"Michel Dänzer" <mdaenzer@redhat.com>,
"DRI Development" <dri-devel@lists.freedesktop.org>,
"xaver.hugl@gmail.com" <xaver.hugl@gmail.com>,
"Melissa Wen" <mwen@igalia.com>,
"Jonas Ådahl" <jadahl@redhat.com>,
"Uma Shankar" <uma.shankar@intel.com>,
"Victoria Brekenfeld" <victoria@system76.com>,
"Aleix Pol" <aleixpol@kde.org>,
"Sebastian Wick" <sebastian.wick@redhat.com>,
"Joshua Ashton" <joshua@froggi.es>
Subject: Re: [RFC] Plane color pipeline KMS uAPI
Date: Mon, 8 May 2023 14:10:46 -0400 [thread overview]
Message-ID: <4b7ccbb1-8a5a-3c7f-cb81-bf0cfd75739a@amd.com> (raw)
In-Reply-To: <CAKMK7uEk=+YyLJOteDjm6mK315ps=wTsJDY3NZdD_N5vpjL=bw@mail.gmail.com>
On 5/8/23 05:18, Daniel Vetter wrote:
> On Mon, 8 May 2023 at 10:58, Simon Ser <contact@emersion.fr> wrote:
>>
>> On Friday, May 5th, 2023 at 21:53, Daniel Vetter <daniel@ffwll.ch> 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@ffwll.ch> 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 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 solve.
>
> _Way_ too much graph compared to what I'm proposing :-)
>
> Also I guess what's not clear: This is 100% a bikeshed with no impact
> on the actual color handling pipeline in any semantic way. At all. If
> you think it is, it's not what I mean.
>
> I guess the misunderstanding started out with me asking for "graph
> nodes" and you thinking "full blown graph structure with mandatory
> flexibility". I really only wanted to bring up the slightly more
> generic "node" think, and you can totally think of them as "list
> nodes" in the context of color op pipelines.
> -Daniel
next prev parent reply other threads:[~2023-05-08 18:10 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-04 15:22 [RFC] Plane color pipeline KMS uAPI Simon Ser
2023-05-04 21:10 ` Harry Wentland
2023-05-05 11:41 ` Pekka Paalanen
2023-05-05 13:30 ` Joshua Ashton
2023-05-05 14:16 ` Pekka Paalanen
2023-05-05 17:01 ` Joshua Ashton
2023-05-09 11:23 ` Melissa Wen
2023-05-09 11:47 ` Pekka Paalanen
2023-05-09 17:01 ` Melissa Wen
2023-05-11 21:21 ` Simon Ser
2023-05-05 15:28 ` Daniel Vetter
2023-05-05 15:57 ` Sebastian Wick
2023-05-05 19:51 ` Daniel Vetter
2023-05-08 8:24 ` Pekka Paalanen
2023-05-08 9:00 ` Daniel Vetter
2023-05-05 16:06 ` Simon Ser
2023-05-05 19:53 ` Daniel Vetter
2023-05-08 8:58 ` Simon Ser
2023-05-08 9:18 ` Daniel Vetter
2023-05-08 18:10 ` Harry Wentland [this message]
[not found] ` <20230508185409.07501f40@n2pa>
2023-05-09 8:17 ` Pekka Paalanen
2023-05-05 20:40 ` Dave Airlie
2023-05-05 22:20 ` Sebastian Wick
2023-05-07 23:14 ` Dave Airlie
2023-05-08 9:37 ` Pekka Paalanen
2023-05-08 10:03 ` Jonas Ådahl
2023-05-09 14:31 ` Harry Wentland
2023-05-09 19:53 ` Dave Airlie
2023-05-09 20:22 ` Simon Ser
2023-05-10 7:59 ` Jonas Ådahl
2023-05-10 8:59 ` Pekka Paalanen
2023-05-11 9:51 ` Karol Herbst
2023-05-11 16:56 ` Joshua Ashton
2023-05-11 18:56 ` Jonas Ådahl
2023-05-11 19:29 ` Simon Ser
2023-05-12 7:24 ` Pekka Paalanen
2023-05-10 8:48 ` Pekka Paalanen
[not found] ` <20230505160435.6e3ffa4a@n2pa>
2023-05-08 8:49 ` Pekka Paalanen
2023-05-09 8:04 ` Pekka Paalanen
[not found] ` <4341dac6-ada1-2a75-1c22-086d96408a85@quicinc.com>
2023-06-09 15:52 ` Christopher Braga
2023-06-09 16:30 ` Simon Ser
2023-06-09 23:11 ` Christopher Braga
2023-06-12 9:21 ` Pekka Paalanen
2023-06-12 16:56 ` Christopher Braga
2023-06-13 8:23 ` Pekka Paalanen
2023-06-13 16:29 ` Christopher Braga
2023-06-14 9:00 ` Pekka Paalanen
2023-06-15 21:44 ` Christopher Braga
2023-06-16 7:59 ` Pekka Paalanen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4b7ccbb1-8a5a-3c7f-cb81-bf0cfd75739a@amd.com \
--to=harry.wentland@amd.com \
--cc=aleixpol@kde.org \
--cc=contact@emersion.fr \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=jadahl@redhat.com \
--cc=joshua@froggi.es \
--cc=mdaenzer@redhat.com \
--cc=mwen@igalia.com \
--cc=pekka.paalanen@collabora.com \
--cc=sebastian.wick@redhat.com \
--cc=uma.shankar@intel.com \
--cc=victoria@system76.com \
--cc=wayland-devel@lists.freedesktop.org \
--cc=xaver.hugl@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).