Re: [PATCH v3] Documentation: gpu: Mention the requirements for new properties

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5501 bytes --]

On Fri, 11 Jun 2021 13:03:09 +0100
Liviu Dudau <liviu.dudau@arm.com> wrote:

> On Fri, Jun 11, 2021 at 08:14:59AM +0000, Simon Ser wrote:
> > On Thursday, June 10th, 2021 at 23:00, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >   
> > > If there's a strong consensus that we really need this then I'm not
> > > going to nack this, but this really needs a pile of acks from
> > > compositor folks that they're willing to live with the resulting
> > > fallout this will likely bring. Your cc list seems to have an absence
> > > of compositor folks, but instead every driver maintainer. That's
> > > backwards. We make uapi for userspace, not for kernel driver
> > > maintainers!  
> > 
> > In wlroots we have a policy of only allowing standard KMS properties to
> > be used. Any vendor-specific property is going to be less well-defined,
> > less widely useful, potentially have more design issues, potentially
> > overlap in functionality with other vendor-specific properties, likely
> > have some hardware-specific assumptions, etc.
> > 
> > What matters here is discussing with other driver & user-space folks to
> > make sure the new property's design is sound. Designing uAPI is hard.
> > 
> > If kernel folks are struggling with a user-space implementation, they
> > should discuss with user-space folks to see which project would be
> > interested. There's a chance a compositor will be interested in the new
> > property and will just do the user-space part for you, if not we can
> > suggest candidate projects.
> > 
> > tl;dr strong agree with Daniel here.  
> 
> I think the assumption you and Daniel are making is that the first implementation of
> a new KMS property can be made standard from day one and that it will work for any
> late comer driver as is, without having to make changes to its behaviour in a
> significant way. In my experience that is not the case.
> 
> I think we have moved from the times when we were trying to implement in the Linux
> world features that were available in the hardware but needed a kernel and userspace
> API. The set of properties that exist in KMS cover a lot of needed functionality and
> I don't expect to see new properties for stuff that is already supported by hardware.
> 
> What I'm expected to see in the future is new functionality that gets implemented by
> one hardware vendor and the kernel developers trying to enable that for userspace. It
> could be that the new property is generic, but there is no way of testing that on
> more than one implementation yet, so I'd say we are generous calling it "standard
> property". When the second or third hardware vendor comes along and starts supporting
> that property with their own set of extra requirements, then we can call it
> "standard".

I agree that is a problem with trying to make generic anything. But it
does not mean you should not even try. Maybe trying really hard saves a
couple revisions.

What I think should be planned for is revisions. How to add new
properties that do the same thing but better, while documenting that a
userspace KMS client can use only one revision at a time. You never
remove old revisions, unless maybe with a DRM client cap they
could disappear from that file description if that is necessary for
seeing the new revision.

While designing this, one also needs to take into account that KMS
clients need to be able to save and restore properties *they do not
understand*. So exposing two revisions of the same feature
simultaneously would break save/restore is that's an error.

> Then comes the effort cost: would it be easier to start with a vendor
> property that only the vendor needs to support (and can submit patches into the
> compositors to do so) and when the standard property gets added moves to that, or

But you can't move, you can only add? You can't delete the old property
in kernel if it was ever released with a kernel and anyone used it. In
the same sentence you also imply that there is a user of it, so
removing it will break that user. Then you'll have to track the
userspace lifetime to figure out which decade you can try removing it.

> should we start with a generic property that gets implemented by the compositors
> (maybe, but then only one vendor supports it) and then later when we actually
> standardise the property we will have to carry backwards compatibility code in the
> kernel to handle the old behaviour for old userspace? My proposal to Maxime was for
> the former option to be reflected in the documentation, but I would like to hear your
> thoughts.

You have to carry the backward compatibility in all cases, right?

Userspace OTOH can drop support for older less supported KMS properties
while taking advantage of a new revision. Userspace is not required to
support old kernels forever.


Here's a wild counter-proposal off a tangent:

How about we make "implemented in and testable with VKMS" the rule,
instead of "is generic" for new properties?

VKMS is what compositors (will) use in CI. I would feel hugely less bad
about using a property that only one hardware driver ever implements,
if also VKMS implements it in a way that compositor CI can observe it
working.

I don't expect this proposal to be accepted, but it's food for thought.
The major problem for compositor projects is testing as you usually
don't have the hardware, IMO. CI tends to not have any hardware.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]