Re: [PATCH 5/5] drm/i915: Replace "Broadcast RGB" with "RGB quantization range" property

From: Yussuf Khalil <dev@pp3345.net>
To: Daniel Vetter <daniel@ffwll.ch>,
	Jani Nikula <jani.nikula@linux.intel.com>
Cc: Simon Ser <contact@emersion.fr>, David Airlie <airlied@linux.ie>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>
Subject: Re: [PATCH 5/5] drm/i915: Replace "Broadcast RGB" with "RGB quantization range" property
Date: Tue, 14 Apr 2020 23:11:07 +0200	[thread overview]
Message-ID: <2cfe44c96818515939050ad19e9c248e50519be2.camel@pp3345.net> (raw)
In-Reply-To: <20200414123404.GT3456981@phenom.ffwll.local>

On Tue, 2020-04-14 at 14:34 +0200, Daniel Vetter wrote:
> On Tue, Apr 14, 2020 at 02:21:06PM +0300, Jani Nikula wrote:
> > On Tue, 14 Apr 2020, Jani Nikula <jani.nikula@linux.intel.com>
> > wrote:
> > > On Mon, 13 Apr 2020, Simon Ser <contact@emersion.fr> wrote:
> > > > On Monday, April 13, 2020 11:40 PM, Yussuf Khalil <
> > > > dev@pp3345.net> wrote:
> > > > 
> > > > > DRM now has a globally available "RGB quantization range"
> > > > > connector
> > > > > property. i915's "Broadcast RGB" that fulfils the same
> > > > > purpose is now
> > > > > considered deprecated, so drop it in favor of the DRM
> > > > > property.
> > > > 
> > > > For a UAPI point-of-view, I'm not sure this is fine. Some user-
> > > > space
> > > > might depend on this property, dropping it would break such
> > > > user-space.
> > > 
> > > Agreed.
> > > 
> > > > Can we make this property deprecated but still keep it for
> > > > backwards
> > > > compatibility?
> > > 
> > > Would be nice to make the i915 specific property an "alias" for
> > > the new
> > > property, however I'm not sure how you'd make that happen.
> > > Otherwise
> > > juggling between the two properties is going to be a nightmare.
> > 
> > Ah, the obvious easy choice is to use the property and enum names
> > already being used by i915 and gma500, and you have no problem.
> > Perhaps
> > they're not the names you'd like, but then looking at the total
> > lack of
> > consistency across property naming makes them fit right in. ;)
> 
> Yeah if we don't have contradictory usage across drivers when
> modernizing
> these properties, then let's just stick with the names already there.
> It's
> not pretty, but works better since more userspace/internet howtos
> know how
> to use this stuff.
> -Daniel

Note that i915's "Broadcast RGB" isn't the same as gma500's: i915 has an
"Automatic" option, whereas gma500 does not. Also, radeon has a property called
"output_csc" that fulfills a similar purpose. Looking at the code, though, it
seems that radeon does not adhere to the standard correctly (or I am missing
something).

An alternative would be to leave the existing driver-specific properties and
change them to be pseudo-aliases for the "RGB quantization range" property.
This can be done by letting the drivers read from and write to the new property
when user-space tries to read or modify the driver's property. This way we could
retain full backwards compatibility for all drivers equally.

What do you think?

Regards
Yussuf