| Message ID | 20240209165307.29856-1-xaver.hugl@kde.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: document userspace expectations around the Colorspace connector property | expand |
On Fri, Feb 09, 2024 at 05:53:07PM +0100, Xaver Hugl wrote: > Signed-off-by: Xaver Hugl <xaver.hugl@kde.org> > --- > drivers/gpu/drm/drm_connector.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index b0516505f7ae..01e13984629b 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -2158,6 +2158,14 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); > * one supported. Sink supported colorspaces should be retrieved by > * userspace from EDID and driver will not explicitly expose them. > * > + * As userspace can't currently know whether or not the output is using > + * RGB or YCC signalling, the driver must translate properties to their > + * relevant RGB or YCC counterparts, depending on the actually used > + * signalling. Property values that are only valid for either YCC or RGB > + * and have no equivalent for the other signalling type must not be > + * exposed as supported, unless the driver can guarantee it never uses > + * the signalling that doesn't match the property. > + * > * Basically the expectation from userspace is: > * - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink > * colorspace > -- > 2.43.0 > Thank you! Reviewed-by: Sebastian Wick <sebastian.wick@redhat.com>
On Fri, 9 Feb 2024 17:53:07 +0100 Xaver Hugl <xaver.hugl@kde.org> wrote: > Signed-off-by: Xaver Hugl <xaver.hugl@kde.org> > --- > drivers/gpu/drm/drm_connector.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index b0516505f7ae..01e13984629b 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -2158,6 +2158,14 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); > * one supported. Sink supported colorspaces should be retrieved by > * userspace from EDID and driver will not explicitly expose them. > * > + * As userspace can't currently know whether or not the output is using > + * RGB or YCC signalling, the driver must translate properties to their > + * relevant RGB or YCC counterparts, depending on the actually used > + * signalling. Property values that are only valid for either YCC or RGB > + * and have no equivalent for the other signalling type must not be > + * exposed as supported, unless the driver can guarantee it never uses > + * the signalling that doesn't match the property. > + * > * Basically the expectation from userspace is: > * - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink > * colorspace Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com> Thanks, pq
On Fri, 9 Feb 2024 17:53:07 +0100 Xaver Hugl <xaver.hugl@kde.org> wrote: > Signed-off-by: Xaver Hugl <xaver.hugl@kde.org> > --- > drivers/gpu/drm/drm_connector.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index b0516505f7ae..01e13984629b 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -2158,6 +2158,14 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); > * one supported. Sink supported colorspaces should be retrieved by > * userspace from EDID and driver will not explicitly expose them. > * > + * As userspace can't currently know whether or not the output is using > + * RGB or YCC signalling, the driver must translate properties to their > + * relevant RGB or YCC counterparts, depending on the actually used > + * signalling. Property values that are only valid for either YCC or RGB > + * and have no equivalent for the other signalling type must not be > + * exposed as supported, unless the driver can guarantee it never uses > + * the signalling that doesn't match the property. > + * > * Basically the expectation from userspace is: > * - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink > * colorspace While this addition is good, I have another question: Does "Colorspace" property choose also the RGB->YCbCr matrix that the driver will use when it happens to use YCbCr signalling? So far we have only been talking about the primaries and white point.
On Mon, Feb 12, 2024 at 11:10:15AM +0200, Pekka Paalanen wrote: > On Fri, 9 Feb 2024 17:53:07 +0100 > Xaver Hugl <xaver.hugl@kde.org> wrote: > > > Signed-off-by: Xaver Hugl <xaver.hugl@kde.org> > > --- > > drivers/gpu/drm/drm_connector.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > > index b0516505f7ae..01e13984629b 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -2158,6 +2158,14 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); > > * one supported. Sink supported colorspaces should be retrieved by > > * userspace from EDID and driver will not explicitly expose them. > > * > > + * As userspace can't currently know whether or not the output is using > > + * RGB or YCC signalling, the driver must translate properties to their > > + * relevant RGB or YCC counterparts, depending on the actually used > > + * signalling. Property values that are only valid for either YCC or RGB > > + * and have no equivalent for the other signalling type must not be > > + * exposed as supported, unless the driver can guarantee it never uses > > + * the signalling that doesn't match the property. > > + * > > * Basically the expectation from userspace is: > > * - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink > > * colorspace > > While this addition is good, I have another question: > > Does "Colorspace" property choose also the RGB->YCbCr matrix that the > driver will use when it happens to use YCbCr signalling? > > So far we have only been talking about the primaries and white point. Uh, yeah, good point. The InfoFrames do affect both the YCbCr conversion and the transfer characteristics that the sink expects. Drivers should do the RGB to YCbCr conversion with the new matrix. I think (and very much hope) that drivers don't rely on the TF for any internal processing but if they did, they also should use the one the sink expects.
On Mon, 12 Feb 2024 17:50:36 +0100 Sebastian Wick <sebastian.wick@redhat.com> wrote: > On Mon, Feb 12, 2024 at 11:10:15AM +0200, Pekka Paalanen wrote: > > On Fri, 9 Feb 2024 17:53:07 +0100 > > Xaver Hugl <xaver.hugl@kde.org> wrote: > > > > > Signed-off-by: Xaver Hugl <xaver.hugl@kde.org> > > > --- > > > drivers/gpu/drm/drm_connector.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > > > index b0516505f7ae..01e13984629b 100644 > > > --- a/drivers/gpu/drm/drm_connector.c > > > +++ b/drivers/gpu/drm/drm_connector.c > > > @@ -2158,6 +2158,14 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); > > > * one supported. Sink supported colorspaces should be retrieved by > > > * userspace from EDID and driver will not explicitly expose them. > > > * > > > + * As userspace can't currently know whether or not the output is using > > > + * RGB or YCC signalling, the driver must translate properties to their > > > + * relevant RGB or YCC counterparts, depending on the actually used > > > + * signalling. Property values that are only valid for either YCC or RGB > > > + * and have no equivalent for the other signalling type must not be > > > + * exposed as supported, unless the driver can guarantee it never uses > > > + * the signalling that doesn't match the property. > > > + * > > > * Basically the expectation from userspace is: > > > * - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink > > > * colorspace > > > > While this addition is good, I have another question: > > > > Does "Colorspace" property choose also the RGB->YCbCr matrix that the > > driver will use when it happens to use YCbCr signalling? > > > > So far we have only been talking about the primaries and white point. > > Uh, yeah, good point. The InfoFrames do affect both the YCbCr conversion > and the transfer characteristics that the sink expects. Drivers should > do the RGB to YCbCr conversion with the new matrix. I think (and very > much hope) that drivers don't rely on the TF for any internal processing > but if they did, they also should use the one the sink expects. Oh, is transfer function also somehow associated here? That would be a third thing. Are there any "Colorspace" values that target a display with not gamma 2.2 TF (possibly extended/mirrored)? The only TF metadata control I've known so far is the one in HDR_OUTPUT_METADATA. Thanks, pq
On Mon, Feb 12, 2024 at 05:50:36PM +0100, Sebastian Wick wrote: > On Mon, Feb 12, 2024 at 11:10:15AM +0200, Pekka Paalanen wrote: > > On Fri, 9 Feb 2024 17:53:07 +0100 > > Xaver Hugl <xaver.hugl@kde.org> wrote: > > > > > Signed-off-by: Xaver Hugl <xaver.hugl@kde.org> > > > --- > > > drivers/gpu/drm/drm_connector.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > > > index b0516505f7ae..01e13984629b 100644 > > > --- a/drivers/gpu/drm/drm_connector.c > > > +++ b/drivers/gpu/drm/drm_connector.c > > > @@ -2158,6 +2158,14 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); > > > * one supported. Sink supported colorspaces should be retrieved by > > > * userspace from EDID and driver will not explicitly expose them. > > > * > > > + * As userspace can't currently know whether or not the output is using > > > + * RGB or YCC signalling, the driver must translate properties to their > > > + * relevant RGB or YCC counterparts, depending on the actually used > > > + * signalling. Property values that are only valid for either YCC or RGB > > > + * and have no equivalent for the other signalling type must not be > > > + * exposed as supported, unless the driver can guarantee it never uses > > > + * the signalling that doesn't match the property. > > > + * > > > * Basically the expectation from userspace is: > > > * - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink > > > * colorspace > > > > While this addition is good, I have another question: > > > > Does "Colorspace" property choose also the RGB->YCbCr matrix that the > > driver will use when it happens to use YCbCr signalling? > > > > So far we have only been talking about the primaries and white point. > > Uh, yeah, good point. The InfoFrames do affect both the YCbCr conversion > and the transfer characteristics that the sink expects. Drivers should > do the RGB to YCbCr conversion with the new matrix. I think (and very > much hope) that drivers don't rely on the TF for any internal processing > but if they did, they also should use the one the sink expects. What we need is: - list of the property values that are still allowd (the ones that still make sense with the changed semantics) - mark all the other ones as deprecated - filter/reject the deprecated values when creating the property - document each valid enum value fully (for both RGB and YCbCr output): * what exacly is signalled to the sink via infoframe/msa/vsc sdp * what processing is the driver expected to perform on the actual data
On Tue, Feb 13, 2024 at 11:26:44AM +0200, Ville Syrjälä wrote: > On Mon, Feb 12, 2024 at 05:50:36PM +0100, Sebastian Wick wrote: > > On Mon, Feb 12, 2024 at 11:10:15AM +0200, Pekka Paalanen wrote: > > > On Fri, 9 Feb 2024 17:53:07 +0100 > > > Xaver Hugl <xaver.hugl@kde.org> wrote: > > > > > > > Signed-off-by: Xaver Hugl <xaver.hugl@kde.org> > > > > --- > > > > drivers/gpu/drm/drm_connector.c | 8 ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > > > > index b0516505f7ae..01e13984629b 100644 > > > > --- a/drivers/gpu/drm/drm_connector.c > > > > +++ b/drivers/gpu/drm/drm_connector.c > > > > @@ -2158,6 +2158,14 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); > > > > * one supported. Sink supported colorspaces should be retrieved by > > > > * userspace from EDID and driver will not explicitly expose them. > > > > * > > > > + * As userspace can't currently know whether or not the output is using > > > > + * RGB or YCC signalling, the driver must translate properties to their > > > > + * relevant RGB or YCC counterparts, depending on the actually used > > > > + * signalling. Property values that are only valid for either YCC or RGB > > > > + * and have no equivalent for the other signalling type must not be > > > > + * exposed as supported, unless the driver can guarantee it never uses > > > > + * the signalling that doesn't match the property. > > > > + * > > > > * Basically the expectation from userspace is: > > > > * - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink > > > > * colorspace > > > > > > While this addition is good, I have another question: > > > > > > Does "Colorspace" property choose also the RGB->YCbCr matrix that the > > > driver will use when it happens to use YCbCr signalling? > > > > > > So far we have only been talking about the primaries and white point. > > > > Uh, yeah, good point. The InfoFrames do affect both the YCbCr conversion > > and the transfer characteristics that the sink expects. Drivers should > > do the RGB to YCbCr conversion with the new matrix. I think (and very > > much hope) that drivers don't rely on the TF for any internal processing > > but if they did, they also should use the one the sink expects. > > What we need is: > - list of the property values that are still allowd (the ones that still > make sense with the changed semantics) > - mark all the other ones as deprecated > - filter/reject the deprecated values when creating the property > - document each valid enum value fully (for both RGB and YCbCr output): > * what exacly is signalled to the sink via infoframe/msa/vsc sdp > * what processing is the driver expected to perform on the actual data I'm starting to agree that this is a better idea than coming up with a number of rules. We only rely on the default colorimetry as indicated by EDID and BT.2020. Xaver, are you going to update the patch accordingly? > -- > Ville Syrjälä > Intel >
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index b0516505f7ae..01e13984629b 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2158,6 +2158,14 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); * one supported. Sink supported colorspaces should be retrieved by * userspace from EDID and driver will not explicitly expose them. * + * As userspace can't currently know whether or not the output is using + * RGB or YCC signalling, the driver must translate properties to their + * relevant RGB or YCC counterparts, depending on the actually used + * signalling. Property values that are only valid for either YCC or RGB + * and have no equivalent for the other signalling type must not be + * exposed as supported, unless the driver can guarantee it never uses + * the signalling that doesn't match the property. + * * Basically the expectation from userspace is: * - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink * colorspace
Signed-off-by: Xaver Hugl <xaver.hugl@kde.org> --- drivers/gpu/drm/drm_connector.c | 8 ++++++++ 1 file changed, 8 insertions(+)