DRM COLOR_RANGE property

Thu Jul 5 14:05:26 UTC 2018

On Wed, Jul 04, 2018 at 01:06:42PM +0200, Daniel Vetter wrote:
> On Wed, Jul 4, 2018 at 11:58 AM, Russell King - ARM Linux
> <linux at armlinux.org.uk> wrote:
> > On Wed, Jul 04, 2018 at 11:24:10AM +0200, Daniel Vetter wrote:
> >> On Wed, Jul 04, 2018 at 10:05:41AM +0100, Russell King - ARM Linux wrote:
> >> > On Wed, Jul 04, 2018 at 10:26:04AM +0200, Daniel Vetter wrote:
> >> > > On Tue, Jul 03, 2018 at 05:18:57PM +0100, Russell King - ARM Linux wrote:
> >> > > > Can someone provide a deeper explanation about exactly what this
> >> > > > property represents please?
> >> > > >
> >> > > > Does this represent the range of YCbCr values _into_ a YCbCr-to-RGB
> >> > > > conversion (in other words, the range of values in the framebuffer),
> >> > > > or the expected output range from the conversion?
> >> > > >
> >> > > > This matters, because a "limited", (iow, eg, BT601 compliant YCbCr)
> >> > > > framebuffer where the Y signal is between 16..235 being displayed
> >> > > > on a full-range RGB output would need different conversion from
> >> > > > needing a limited-range RGB output.
> >> > > >
> >> > > > If it is indeed the output, then why is this a property of the plane?
> >> > > > Is that not a property of:
> >> > > >
> >> > > > (a) whether the plane is being blended or overlaid onto a graphics
> >> > > >     plane which uses full-range RGB
> >> > > > (b) the properties of the output(s) to which the plane is being
> >> > > >     displayed.
> >> > > >
> >> > > > IOW, it seems that the output of the CSC is more to do with what's
> >> > > > downstream of the plane than with the plane itself.
> >> > > >
> >> > > > For example, take this situation:
> >> > > >
> >> > > > plane 0 - graphics, full range RGB
> >> > > >                                   >-- CRTC --> HDMI sink only supporting
> >> > > > plane 1 - video, limited range YUV              limited range RGB
> >> > > >
> >> > > > In order to display the graphics correctly in that scenario, the HDMI
> >> > > > output needs to compress the RGB 0-255 range down to 16..236 to be
> >> > > > compliant.  If the video is limited range, and the CSC produces a
> >> > > > limited range RGB output, then plane 1 gets its range further
> >> > > > compressed at the HDMI output, which surely is undesirable.
> >> > > >
> >> > > > It would surely be better, if it's not possible to map the range of
> >> > > > plane 0 to limited range, to instead expand the YUV range and then
> >> > > > recompress it at the HDMI output to match the capabilities of the
> >> > > > attached source.
> >> > > >
> >> > > > It also seems logical that describing the range of the RGB plane would
> >> > > > also be sane - if the application is limiting graphics RGB to 16..235,
> >> > > > then you'd want the CSC output to do the same and there'd be no need
> >> > > > for any range expansion or compression.
> >> > > >
> >> > > > I'd personally like drm_plane_create_color_properties() to allow
> >> > > > creation of COLOR_ENCODING without COLOR_RANGE (iow, supported_ranges
> >> > > > being zero) until COLOR_RANGE is better defined than it is at present.
> >> > > >
> >> > > > Thoughts?
> >> > > >
> >> > > > I'm bringing this up, because the hardware I have has a CSC that
> >> > > > accepts BT601 and BT709 formats, controlled by a single bit.  Another
> >> > > > bit controls whether the CSC produces 0..255 output or 16..235 output.
> >> > > > That is then blended/overlaid with the graphics plane (0..255) and
> >> > > > sent to the output.  Having a "limited range" YUV plane produce
> >> > > > 16..235 range output makes it look low-contrast compared to the
> >> > > > graphics, which is what would be expected - "16" is not black
> >> > > > compared to the black of the graphics in the same way that "235" is
> >> > > > not white compared to the graphics.
> >> > >
> >> > > Drivers are supposed to automatically figure this out by looking at the
> >> > > edid. In i915 we also allow userspace to override this with the "Broadcast
> >> > > RGB" property on the connector. Unfortunately we haven't polished that
> >> > > property yet (not sure what other drivers are doing tbh), so it's only
> >> > > listed in the Documentation/gpu/kms-properties.csv graveyard :-/
> >> >
> >> > In which case, I'd like to implement the COLOR_ENCODING property but
> >> > not the COLOR_RANGE property until COLOR_RANGE is better defined.
> >> > Unfortunately, at the moment the choices are to have either both
> >> > properties or no properties - drm_plane_create_color_properties()
> >> > doesn't support only creating the encoding property.
> >> >
> >> > Implementing it without it being well defined is a recipe for having
> >> > a broken UAPI.  So, I propose:
> >>
> >> Hm maybe I misunderstood, but I thought the COLOR_RANGE is on the input
> >> side.
> >
> > If that's the case, I should force it to only indicate support for
> > limited range, while programming the CSC to produce full range RGB
> > on its output (see below).
> 
> Agreed (as far as my understanding of all this goes at least).
> 
> >> And you need to adjust your CSC to make sure the output range is
> >> whatever makes things Just Work. Probably better to poke Ville on these
> >> details, and then clarify the docs.
> >
> > It's not possible to arbitarily program the CSC - I have two bits:
> >
> > - one bit selects whether BT601 or BT709 YUV encoding is used on the CSC
> >   input.
> > - one bit selects whether limited or full range RGB is produced on the
> >   CSC output.
> >
> > Given that I have to assume that the primary plane contains full range
> > RGB values (DRM has no means to communicate from userspace anything
> > else at present) I have to always select full range RGB at the CSC
> > output to be blended with the primary plane.
> 
> You could attach the color range stuff also to the primary plane, but
> I don't think any current userspace is prepared to use it.
> 
> > I then need some way to know that the TDA998x is being fed by a full
> > range RGB (for the time being, I'm going to assume that's the case)
> > and convert to limited range in the TDA998x if the sink doesn't
> > support full range.  I'm proposing to implement your i915 "broadcast
> > rgb" control in TDA998x to allow that to be overridden - maybe that
> > should become a standardised control for HDMI outputs?
> 
> Yup that sounds like a very good idea, plus documenting it in
> 
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#hdmi-specific-connector-properties

Note that at some point we'll have to come up with some new props for
specifying the full output colorimetry information and I have a
feeling the broadcast rgb prop may get slightly in the way. But
we'll have to cross that bridge in i915 anyway so I guess it wouldn't
complicate the situation any more if other drivers have this prop as
well.

-- 
Ville Syrjälä
Intel