On Mon, 27 May 2024 at 16:30, Maxime Ripard <mrip...@kernel.org> wrote:
>
> Hi,
>
> On Mon, May 27, 2024 at 12:43:18PM GMT, Dmitry Baryshkov wrote:
> > 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 <dave.steven...@raspberrypi.com>
> > > > > Acked-by: Pekka Paalanen <pekka.paala...@collabora.com>
> > > > > Reviewed-by: Sebastian Wick <sebastian.w...@redhat.com>
> > > > > Signed-off-by: Maxime Ripard <mrip...@kernel.org>
> > > > > ---
> > > > > 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
>
> That's not really what I meant: that file is deprecated now and it's not
> where we document the properties anymore. This patch has improved that
> documentation and moved it to the new place, and removed the deprecated
> part.
>
> However, this line shouldn't be there at all. I'll add a patch to remove
> it.
SG
>
> Thanks!
> Maxime
--
With best wishes
Dmitry
