[PATCH 2/2] drm/doc: emphasize difference between plane formats and IN_FORMATS blob

[Daniel Vetter]

On Thu, Apr 08, 2021 at 12:59:19PM +0300, Pekka Paalanen wrote:
> On Thu, 08 Apr 2021 08:39:10 +0000
> Simon Ser <contact at emersion.fr> wrote:
> 
> > On Wednesday, April 7th, 2021 at 3:51 PM, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> > 
> > > > +	 * To find out the list of formats that support modifiers, userspace
> > > > +	 * must use the plane IN_FORMATS blob property.
> > > >  	 */  
> > >
> > > Addfb2+modifiers predates the IN_FORMATS blob, so this doesn't
> > > match reality.  
> > 
> > TBH, I'm inclined not to care about this edge-case. It's already
> > complicated enough for user-space to figure out what's the right thing
> > to do when supporting both implicit modifiers and explicit modifiers.
> > Using modifiers without IN_FORMATS is risky, since a whole part of the
> > modifier negotiation mechanism is missing.
> > 
> > Maybe we can just stick a "since kernel x.y.z" in here to address your
> > concern.
> 
> Yeah, or we could word it less "must", e.g. "The list of supported
> formats with their explicit modifiers is advertised via IN_FORMATS blob."
> 
> We don't have to require userspace to not use the explicit modifier
> uAPI if IN_FORMATS does not exist. There might be other ways how
> userspace determines the explicit modifiers, like a Wayland compositor
> advertising those format-modifier pairs that EGL can import. Then
> clients use those, and the compositor can try to import those to KMS as
> well. Maybe it works, maybe it doesn't (the same even if IN_FORMATS
> exists).
> 
> IN_FORMATS just gives better chances of picking something that ends up
> working.

Yup.

> The thing userspace *must not* do is to try to use the no-modifiers uAPI
> when it has an explicit modifier. But do we then have exceptions for
> MOD_LINEAR?
> 
> If a buffer has been allocated with explicit modifier MOD_LINEAR, does
> it mean it is ok to import to KMS using the no-modifiers uAPI or not?

Maybe it work with current userspace, but not guaranteed. I think we
should strongly discourage this at least.

The case this can go boom is if userspace allocates a big bo, with some
implicit metadata. And then suballocates some linear buffer out of that
with explicit modifiers. This could happen if userspace does realize
modifiers work, and then does some funky optimization to linear e.g. as
part of a resolve pass. Not such hw/driver currently exists, but not
something I'd guarantee never happens.

If you then create a drm_fb with that with no modifier specified, you get
the implicit one from the metadata.
> 
> The other things userspace *must not* do is use the explicit modifier
> uAPI when it does not have an explicit modifier, in essence pulling a
> modifier out of a hat. It might be tempting to use MOD_LINEAR in that
> case, sometimes it might work, but it's wrong. Userspace must use the
> no-modifiers uAPI instead.

Yes. Userspace can't guess the modifier if it doesn't have it.


> 
> Right?
> 
> 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.

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

> 
> 
> Thanks,
> pq



> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch