Re: [PATCH] drm: Document requirements for driver-specific KMS props in new drivers

[Sebastian Wick]

On Wed, Mar 06, 2024 at 03:14:15PM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Feb 29, 2024 at 09:28:31PM +0100, Sebastian Wick wrote:
> > When extending support for a driver-specific KMS property to additional
> > drivers, we should apply all the requirements for new properties and
> > make sure the semantics are the same and documented.
> > 
> > Signed-off-by: Sebastian Wick <sebastian.wick@xxxxxxxxxx>
> > ---
> >  Documentation/gpu/drm-kms.rst | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 13d3627d8bc0..afa10a28035f 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -496,6 +496,11 @@ addition to the one mentioned above:
> >  
> >  * An IGT test must be submitted where reasonable.
> >  
> > +For historical reasons, non-standard, driver-specific properties exist. If a KMS
> > +driver wants to add support for one of those properties, the requirements for
> > +new properties apply where possible. Additionally, the documented behavior must
> > +match the de facto semantics of the existing property to ensure compatibility.
> > +
> 
> I'm conflicted about this one, really.
> 
> On one hand, yeah, it's definitely reasonable and something we would
> want on the long run.
> 
> But on the other hand, a driver getting its technical debt worked on for
> free by anyone but its developpers doesn't seem fair to me.

Most of the work would have to be done for a new property as well. The
only additional work is then documenting the de-facto semantics and
moving the existing driver-specific code to the core.

Would it help if we spell out that the developers of the driver-specific
property shall help with both tasks?

> Also, I assume this is in reaction to the discussion we had on the
> Broadcast RGB property. We used in vc4 precisely because there was some
> userspace code to deal with it and we could just reuse it, and it was
> documented. So the requirements were met already.

It was not in drm core and the behavior was not documented properly at
least.

Either way, with Broadcast RGB we were already in a bad situation
because it was implemented by 2 drivers independently. This is what I
want to avoid in the first place. The cleanup afterwards (thank you!)
just exposed the problem.

> Maxime