Hi Sebastian, On Wed, Mar 06, 2024 at 05:50:09PM +0100, Sebastian Wick wrote: > On Wed, Mar 06, 2024 at 03:14:15PM +0100, Maxime Ripard wrote: > > 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. Sure, I think part of the problem with the Broadcast RGB property was also that it hasn't been reviewed by anyone when it was submitted for vc4. > Would it help if we spell out that the developers of the driver-specific > property shall help with both tasks? Yes, that's a good idea, and we should probably require that the maintainers of the driver that first added that property ack the "standardization" work too. > > 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. Actually, I just found out there's three, the third one not being compatible at all with the other two: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/gma500/cdv_device.c#L471 I'll send a patch to figure that one out once the rest will be merged. Maxime
Attachment:
signature.asc
Description: PGP signature
