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

[Laurent Pinchart]

On Mon, Jun 14, 2021 at 04:24:13PM +0100, Liviu Dudau wrote:
> On Mon, Jun 14, 2021 at 05:49:12PM +0300, Pekka Paalanen wrote:
> > On Fri, 11 Jun 2021 13:03:09 +0100
> > Liviu Dudau <liviu.dudau at 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 at 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.
> 
> Agree.
> 
> > 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.
> 
> I quite like the idea of having versions for properties.

It's an interesting idea. We'll need to consider bundles of properties
in that case I think, as in a v1 a feature could be implemented by
properties A, B and C, while in v2 C could be replaced by D and E
(beside A and B themselves also being changed in v2).

> > > 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.
> 
> Not that I am supporting the workflow, but I was trying to address the comments that
> vendors are going to push their own userspace implementation for their vendor
> properties. If that is the case, when they switch to the standard ones they can drop
> the support in userspace for their changes. With the implied assumption that you will
> have fewer vendor implementations hence easier to make changes, KMS properties can be
> deleted if you know there is no user of them (e.g. the vendor has upgraded all their
> software to the standard property).
> 
> > > 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.
> 
> While I don't dislike the proposal (I think it is quite sensible), I am worried that
> for some behaviours VKMS will implement them in a quirky way. To pick (again) the
> example of writeback, real hardware will have a way to tell if the buffer has been
> sent successfully to memory and it might take more than one refresh period, while
> VKMS (if I remember correctly) fakes it and signals the fence at the next vblank. If
> you code your compositor based on VKMS you might get unexpected artifacts on real
> hardware.

I'm also worried about properties that related to hardware processing of
frames, for instance related to colour spaces, or even scaling. VKMS has
a software composer implemented in the kernel, it may get fairly
complicated if we need to implement all kind of processing.

-- 
Regards,

Laurent Pinchart