On Mon, May 27, 2024 at 11:02:13AM +0200, Maxime Ripard wrote:
> Hi,
>
> Thanks again for that thorough review :)
>
> On Thu, May 23, 2024 at 01:22:56PM GMT, Dmitry Baryshkov wrote:
> > On Tue, May 21, 2024 at 12:13:50PM +0200, Maxime Ripard wrote:
> > > The i915 driver has a property to force the RGB range of an HDMI output.
> > > The vc4 driver then implemented the same property with the same
> > > semantics. KWin has support for it, and a PR for mutter is also there to
> > > support it.
> > >
> > > Both drivers implementing the same property with the same semantics,
> > > plus the userspace having support for it, is proof enough that it's
> > > pretty much a de-facto standard now and we can provide helpers for it.
> > >
> > > Let's plumb it into the newly created HDMI connector.
> > >
> > > Reviewed-by: Dave Stevenson <[email protected]>
> > > Acked-by: Pekka Paalanen <[email protected]>
> > > Reviewed-by: Sebastian Wick <[email protected]>
> > > Signed-off-by: Maxime Ripard <[email protected]>
> > > ---
> > > Documentation/gpu/kms-properties.csv | 1 -
> > > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 4 +-
> > > drivers/gpu/drm/drm_atomic.c | 2 +
> > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
> > > drivers/gpu/drm/drm_connector.c | 88
> > > +++++++++++++++++++++++++
> > > include/drm/drm_connector.h | 36 ++++++++++
> > > 6 files changed, 133 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/gpu/kms-properties.csv
> > > b/Documentation/gpu/kms-properties.csv
> > > index 0f9590834829..caef14c532d4 100644
> > > --- a/Documentation/gpu/kms-properties.csv
> > > +++ b/Documentation/gpu/kms-properties.csv
> > > @@ -15,11 +15,10 @@ Owner Module/Drivers,Group,Property
> > > Name,Type,Property Values,Object attached,De
> > > ,,“saturation”,RANGE,"Min=0, Max=100",Connector,TBD
> > > ,,“hue”,RANGE,"Min=0, Max=100",Connector,TBD
> > > ,Virtual GPU,“suggested X”,RANGE,"Min=0,
> > > Max=0xffffffff",Connector,property to suggest an X offset for a connector
> > > ,,“suggested Y”,RANGE,"Min=0, Max=0xffffffff",Connector,property to
> > > suggest an Y offset for a connector
> > > ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9""
> > > }",Connector,TDB
> > > -i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"",
> > > ""Limited 16:235"" }",Connector,"When this property is set to Limited
> > > 16:235 and CTM is set, the hardware will be programmed with the result of
> > > the multiplication of CTM by the limited range matrix to ensure the
> > > pixels normally in the range 0..1.0 are remapped to the range
> > > 16/255..235/255."
> >
> > Should it still be defined as a generic property?
>
> I'm not sure what you mean here, sorry. It's being documented as a
> connector property now, so it's very much still listed as a generic
> property?
I didn't perform my duty well enough and I didn't check the file for
other instances of the property. Now I indeed see a generic "Broadcast
RGB" property, but to me it looks like having a different set of values:
,,"""Broadcast RGB""",ENUM,"{ “off”, “auto”, “on” }",Connector,TBD
--
With best wishes
Dmitry
