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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 14 Jun 2021 19:33:47 +0300
Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:

> 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@xxxxxxx> 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@xxxxxxxx> 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.

Hi,

indeed it will, but what else could one do to test userspace KMS
clients in generic CI where all you can have is virtual hardware? Maybe
in the long run VKMS needs to loop back to a userspace daemon that
implements all the complex processing and returns the writeback result
via VKMS again? That daemon would then need a single upstream, like the
kernel, where it is maintained and correctness verified.

Or an LD_PRELOAD that hijacks all KMS ioctls and implements virtual
stuff in userspace? Didn't someone already have something like that?
It would need to be lifted to be a required part of kernel UAPI
submissions, I suppose like IGT is nowadays.

Requiring a software implementation for testing has another huge
benefit (or disadvantage, depending on who you ask): "hardware does
secret magic" would not fly anymore. The software implementation would
need to deliver the same results, or at least one would need to define
the tolerances for e.g. color conversions. Otherwise userspace
compositor test suites might pass on the software implementation and
fail on hardware implementations. For compositor developers like me
knowing the exact formulas would be a huge benefit as it would allow me
to use KMS to off-load precision-sensitive operations (e.g.
professional color management). Otherwise, compositors probably need a
switch: "high quality color management? Then do not use KMS features."


Thanks,
pq

Attachment: pgpcce_aSOGWW.pgp
Description: OpenPGP digital signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux