Re: [PATCH v3 3/9] drm/vc4: hdmi: Add Broadcast RGB property to allow override of RGB range

[Maxime Ripard]

Hi Hans,

On Thu, Oct 19, 2023 at 10:26:40AM +0200, Hans Verkuil wrote:
> Hi Maxime,
> 
> On 19/10/2023 10:02, Maxime Ripard wrote:
> > Hi,
> > 
> > On Wed, Oct 11, 2023 at 03:23:18PM +0200, Daniel Vetter wrote:
> >> On Mon, 6 Mar 2023 at 11:49, Maxime Ripard <maxime@xxxxxxxxxx> wrote:
> >>>
> >>> From: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> >>>
> >>> Copy Intel's "Broadcast RGB" property semantics to add manual override
> >>> of the HDMI pixel range for monitors that don't abide by the content
> >>> of the AVI Infoframe.
> >>>
> >>> Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> >>> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
> >>
> >> Stumbled over this grepping around, but would have been nice to lift
> >> this into drm code and document the property. It's one of the legacy
> >> ones from the table of horrors after all ...
> >>
> >> Shouldn't be an uapi problem because it's copypasted to much, just not great.
> > 
> > We already discussed it on IRC, but just for the record I have a current
> > series that should address exactly that:
> > 
> > https://lore.kernel.org/dri-devel/20230920-kms-hdmi-connector-state-v2-3-17932daddd7d@xxxxxxxxxx/
> > 
> > Maxime
> 
> I've pasted a snippet from that patch below for a quick review:
> 
> >  /**
> >   * DOC: HDMI connector properties
> >   *
> > + * Broadcast RGB (HDMI Specific):
> 
> Full vs Limited is actually not HDMI specific, DisplayPort can signal this as
> well for whatever it is worth.

Sure, what I (and the original patch I guess) meant is that it's only
ever used on HDMI connectors these days. If that ever changes, then we
can update the doc.

> > + *      Indicates the RGB Range (Full vs Limited) used.
> 
> RGB Range -> RGB Quantization Range
> 
> > + *
> > + *      The value of this property can be one of the following:
> > + *
> > + *      Automatic:
> > + *              RGB Range is selected automatically based on the mode
> > + *              according to the HDMI specifications.
> > + *
> > + *      Full:
> > + *              Full RGB Range is forced.
> > + *
> > + *      Limited 16:235:
> 
> It is very unfortunate that this is called "Limited 16:235" instead of just "Limited"
> since for color component bit depths > 8 these values are different.
> 
> I have no idea if it is possible to add an alias "Limited" that you can use instead.
> In any case, this should document that it works just as well for higher bit depths,
> but with different limits.

I had a look and it doesn't look like the property infrastructure can
deal with aliases. I'll add something in the doc

Thanks!
Maxime

Attachment: signature.asc
Description: PGP signature