On Thu, 8 Apr 2021 13:30:16 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Apr 08, 2021 at 12:59:19PM +0300, Pekka Paalanen wrote:

> > The point of these documentation patches is to establish the convention
> > that:
> > 
> > - drm_mode_get_plane::format_type_ptr is the list of pixel formats that
> >   can work via the no-modifiers uAPI, but says nothing about the
> >   explicit modifiers uAPI.
> > 
> > - IN_FORMATS is a list of format-modifier pairs that can work via the
> >   explicit modifiers API, but says nothing about the no-modifiers uAPI.
> > 
> > Is that a reasonable expectation?  
> 
> I'm not sure. I thought they're the same list underneath in the kernel, at
> least for drivers that do support modifiers. The current wording I think
> suggests more meaning than is actually there.

They may be the same list in the kernel today, but do you want to force
all future drivers and future formats-modifiers to have that too?
Or did the boat sail already?

The existing uAPI considers these two to be independent lists, no
documentation saying otherwise, is there?

Should a kernel driver not have a way to say "this format won't work
via the no-modifiers uAPI"?

The practical consequence in userspace is how should userspace collect
the lists of supported format-modifier pairs, when the kernel has two
independent format lists, one carries modifiers explicitly and the
other does not. The one that carries explicit modifiers cannot denote
"no modifier" AFAIU.

So the "obvious" interpretation in userspace is that:
- the format list that does not carry any modifier information should
  be used with the no-modifiers uAPI, and
- the format list that does carry explicit modifiers should be used
  with the explicit modifiers uAPI.

If you were to say, that if IN_FORMATS exists, use it and ignore the
old no-modifiers format list, then the conclusion in userspace when
IN_FORMATS exists is that you cannot use the no-modifiers uAPI, because
all formats that are listed as supported carry an explicit modifier.

I understand that the format or format-modifier lists are not
authoritative. Formats outside of the lists *could* work. But why would
anyone bother trying something that is not suggested to work?

Or, is the intention such, that all formats in IN_FORMATS list imply
some support through the no-modifiers uAPI too, iff buffer
allocation does not give you an explicit modifier?

Or, should there be an i-g-t test to ensure that both the old and
IN_FORMATS lists have the exact same pixel formats always, carving that
fact into stone and resolving all this ambiguity?

> > Is it also so that passing MOD_INVALID to the explicit modifier uAPI
> > (ADDFB2) is invalid argument? Do we have that documented?  
> 
> We'd need to check that, currently it's an out-of-band flag in the struct.
> Atm DRM_FORMAT_MOD_INVALID is entirely an internal sentinel value to
> denote end-of-array entries.
> 
> In practice it wont pass because we validate the modifiers against the
> advertised list.

Right, so while at it, would be good to document that one cannot
substitute no-modifiers uAPI with explicit modifier uAPI using
MOD_INVALID. This should be documented, because other userspace APIs
have tendency to gravitate towards having just one explicit modifiers
function allowing MOD_INVALID, meaning "no modifier".


Thanks,
pq