[PATCH v2] drm/drm_connector: Document Colorspace property variants

[PATCH v2] drm/drm_connector: Document Colorspace property variants

[Sebastian Wick]

The initial idea of the Colorspace prop was that this maps 1:1 to
InfoFrames/SDP but KMS does not give user space enough information nor
control over the output format to figure out which variants can be used
for a given KMS commit. At the same time, properties like Broadcast RGB
expect full range quantization range being produced by user space from
the CRTC and drivers to convert to the range expected by the sink for
the chosen output format, mode, InfoFrames, etc.

This change documents the reality of the Colorspace property. The
Default variant unfortunately is very much driver specific and not
reflected by the EDID. The BT2020 variants are in active use by generic
compositors which have expectations from the driver about the
conversions it has to do when selecting certain output formats.

Everything else is also marked as undefined. Coming up with valid
behavior that makes it usable from user space and consistent with other
KMS properties for those variants is left as an exercise for whoever
wants to use them.

v2:
 * Talk about "pixel operation properties" that user space configures
 * Mention that user space is responsible for checking the EDID for sink
   support
 * Make it clear that drivers can choose between RGB and YCbCr on their
   own

Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
---
 drivers/gpu/drm/drm_connector.c | 79 +++++++++++++++++++++++++--------
 include/drm/drm_connector.h     |  8 ----
 2 files changed, 61 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b0516505f7ae..65cdcc7d22db 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2147,24 +2147,67 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
  * DOC: standard connector properties
  *
  * Colorspace:
- *     This property helps select a suitable colorspace based on the sink
- *     capability. Modern sink devices support wider gamut like BT2020.
- *     This helps switch to BT2020 mode if the BT2020 encoded video stream
- *     is being played by the user, same for any other colorspace. Thereby
- *     giving a good visual experience to users.
- *
- *     The expectation from userspace is that it should parse the EDID
- *     and get supported colorspaces. Use this property and switch to the
- *     one supported. Sink supported colorspaces should be retrieved by
- *     userspace from EDID and driver will not explicitly expose them.
- *
- *     Basically the expectation from userspace is:
- *      - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
- *        colorspace
- *      - Set this new property to let the sink know what it
- *        converted the CRTC output to.
- *      - This property is just to inform sink what colorspace
- *        source is trying to drive.
+ *	This property is used to inform the driver about the color encoding
+ *	user space configured the pixel operation properties to produce.
+ *	The variants set the colorimetry, transfer characteristics, and which
+ *	YCbCr conversion should be used when necessary.
+ *	The transfer characteristics from HDR_OUTPUT_METADATA takes precedence
+ *	over this property.
+ *	User space always configures the pixel operation properties to produce
+ *	full quantization range data (see the Broadcast RGB property).
+ *
+ *	Drivers inform the sink about what colorimetry, transfer
+ *	characteristics, YCbCr conversion, and quantization range to expect
+ *	(this can depend on the output mode, output format and other
+ *	properties). Drivers also convert the user space provided data to what
+ *	the sink expects.
+ *
+ *	User space has to check if the sink supports all of the possible
+ *	colorimetries that the driver is allowed to pick by parsing the EDID.
+ *
+ *	For historical reasons this property exposes a number of variants which
+ *	result in undefined behavior.
+ *
+ *	Default:
+ *		The behavior is driver-specific.
+ *	BT2020_RGB:
+ *	BT2020_YCC:
+ *		User space configures the pixel operation properties to produce
+ *		RGB content with Rec. ITU-R BT.2020 colorimetry, Rec.
+ *		ITU-R BT.2020 (Table 4, RGB) transfer characteristics and full
+ *		quantization range.
+ *		User space can use the HDR_OUTPUT_METADATA property to set the
+ *		transfer characteristics to PQ (Rec. ITU-R BT.2100 Table 4) or
+ *		HLG (Rec. ITU-R BT.2100 Table 5) in which case, user space
+ *		configures pixel operation properties to produce content with
+ *		the respective transfer characteristics.
+ *		User space has to make sure the sink supports Rec.
+ *		ITU-R BT.2020 R'G'B' and Rec. ITU-R BT.2020 Y'C'BC'R
+ *		colorimetry.
+ *		Drivers can configure the sink to use an RGB format, tell the
+ *		sink to expect Rec. ITU-R BT.2020 R'G'B' colorimetry and convert
+ *		to the appropriate quantization range.
+ *		Drivers can configure the sink to use a YCbCr format, tell the
+ *		sink to expect Rec. ITU-R BT.2020 Y'C'BC'R colorimetry, convert
+ *		to YCbCr using the Rec. ITU-R BT.2020 non-constant luminance
+ *		conversion matrix and convert to the appropriate quantization
+ *		range.
+ *		The variants BT2020_RGB and BT2020_YCC are equivalent and the
+ *		driver chooses between RGB and YCbCr on its own.
+ *	SMPTE_170M_YCC:
+ *	BT709_YCC:
+ *	XVYCC_601:
+ *	XVYCC_709:
+ *	SYCC_601:
+ *	opYCC_601:
+ *	opRGB:
+ *	BT2020_CYCC:
+ *	DCI-P3_RGB_D65:
+ *	DCI-P3_RGB_Theater:
+ *	RGB_WIDE_FIXED:
+ *	RGB_WIDE_FLOAT:
+ *	BT601_YCC:
+ *		The behavior is undefined.
  *
  * Because between HDMI and DP have different colorspaces,
  * drm_mode_create_hdmi_colorspace_property() is used for HDMI connector and
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fe88d7fc6b8f..02c42b01a3a7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -437,14 +437,6 @@ enum drm_privacy_screen_status {
  *
  * DP definitions come from the DP v2.0 spec
  * HDMI definitions come from the CTA-861-H spec
- *
- * A note on YCC and RGB variants:
- *
- * Since userspace is not aware of the encoding on the wire
- * (RGB or YCbCr), drivers are free to pick the appropriate
- * variant, regardless of what userspace selects. E.g., if
- * BT2020_RGB is selected by userspace a driver will pick
- * BT2020_YCC if the encoding on the wire is YUV444 or YUV420.
   *
  * @DRM_MODE_COLORIMETRY_DEFAULT:
  *   Driver specific behavior.
-- 
2.44.0

Re: [PATCH v2] drm/drm_connector: Document Colorspace property variants

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 8024 bytes --]

On Tue,  5 Mar 2024 14:51:49 +0100
Sebastian Wick <sebastian.wick@redhat.com> wrote:

> The initial idea of the Colorspace prop was that this maps 1:1 to
> InfoFrames/SDP but KMS does not give user space enough information nor
> control over the output format to figure out which variants can be used
> for a given KMS commit. At the same time, properties like Broadcast RGB
> expect full range quantization range being produced by user space from
> the CRTC and drivers to convert to the range expected by the sink for
> the chosen output format, mode, InfoFrames, etc.
> 
> This change documents the reality of the Colorspace property. The
> Default variant unfortunately is very much driver specific and not
> reflected by the EDID. The BT2020 variants are in active use by generic
> compositors which have expectations from the driver about the
> conversions it has to do when selecting certain output formats.
> 
> Everything else is also marked as undefined. Coming up with valid
> behavior that makes it usable from user space and consistent with other
> KMS properties for those variants is left as an exercise for whoever
> wants to use them.
> 
> v2:
>  * Talk about "pixel operation properties" that user space configures
>  * Mention that user space is responsible for checking the EDID for sink
>    support
>  * Make it clear that drivers can choose between RGB and YCbCr on their
>    own
> 
> Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 79 +++++++++++++++++++++++++--------
>  include/drm/drm_connector.h     |  8 ----
>  2 files changed, 61 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b0516505f7ae..65cdcc7d22db 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2147,24 +2147,67 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
>   * DOC: standard connector properties
>   *
>   * Colorspace:
> - *     This property helps select a suitable colorspace based on the sink
> - *     capability. Modern sink devices support wider gamut like BT2020.
> - *     This helps switch to BT2020 mode if the BT2020 encoded video stream
> - *     is being played by the user, same for any other colorspace. Thereby
> - *     giving a good visual experience to users.
> - *
> - *     The expectation from userspace is that it should parse the EDID
> - *     and get supported colorspaces. Use this property and switch to the
> - *     one supported. Sink supported colorspaces should be retrieved by
> - *     userspace from EDID and driver will not explicitly expose them.
> - *
> - *     Basically the expectation from userspace is:
> - *      - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> - *        colorspace
> - *      - Set this new property to let the sink know what it
> - *        converted the CRTC output to.
> - *      - This property is just to inform sink what colorspace
> - *        source is trying to drive.
> + *	This property is used to inform the driver about the color encoding
> + *	user space configured the pixel operation properties to produce.
> + *	The variants set the colorimetry, transfer characteristics, and which
> + *	YCbCr conversion should be used when necessary.
> + *	The transfer characteristics from HDR_OUTPUT_METADATA takes precedence
> + *	over this property.
> + *	User space always configures the pixel operation properties to produce
> + *	full quantization range data (see the Broadcast RGB property).
> + *
> + *	Drivers inform the sink about what colorimetry, transfer
> + *	characteristics, YCbCr conversion, and quantization range to expect
> + *	(this can depend on the output mode, output format and other
> + *	properties). Drivers also convert the user space provided data to what
> + *	the sink expects.

Hi Sebastian,

should it be more explicit that drivers are allowed to do only
RGB->YCbCr and quantization range conversions, but not TF nor gamut
conversions?

That is, if the driver cannot pick the TF implied by "Colorspace"
property for the sink, then it cannot pick another TF for the sink and
silently convert. It think this should apply to all options including
the undefined ones. Or is that too much to guess?

> + *
> + *	User space has to check if the sink supports all of the possible
> + *	colorimetries that the driver is allowed to pick by parsing the EDID.

All? Rather than at least one?

Is this how it has been implemented for BT2020, that userspace picked
colorimetry and driver picked color model and quantization are
completely independent, and drivers do not check the combination
against EDID?

If so, "all" it is. Would be good to explain this in the commit message.

> + *
> + *	For historical reasons this property exposes a number of variants which
> + *	result in undefined behavior.
> + *
> + *	Default:
> + *		The behavior is driver-specific.
> + *	BT2020_RGB:
> + *	BT2020_YCC:
> + *		User space configures the pixel operation properties to produce
> + *		RGB content with Rec. ITU-R BT.2020 colorimetry, Rec.
> + *		ITU-R BT.2020 (Table 4, RGB) transfer characteristics and full
> + *		quantization range.
> + *		User space can use the HDR_OUTPUT_METADATA property to set the
> + *		transfer characteristics to PQ (Rec. ITU-R BT.2100 Table 4) or
> + *		HLG (Rec. ITU-R BT.2100 Table 5) in which case, user space
> + *		configures pixel operation properties to produce content with
> + *		the respective transfer characteristics.
> + *		User space has to make sure the sink supports Rec.
> + *		ITU-R BT.2020 R'G'B' and Rec. ITU-R BT.2020 Y'C'BC'R
> + *		colorimetry.
> + *		Drivers can configure the sink to use an RGB format, tell the
> + *		sink to expect Rec. ITU-R BT.2020 R'G'B' colorimetry and convert
> + *		to the appropriate quantization range.
> + *		Drivers can configure the sink to use a YCbCr format, tell the
> + *		sink to expect Rec. ITU-R BT.2020 Y'C'BC'R colorimetry, convert
> + *		to YCbCr using the Rec. ITU-R BT.2020 non-constant luminance
> + *		conversion matrix and convert to the appropriate quantization
> + *		range.
> + *		The variants BT2020_RGB and BT2020_YCC are equivalent and the
> + *		driver chooses between RGB and YCbCr on its own.
> + *	SMPTE_170M_YCC:
> + *	BT709_YCC:
> + *	XVYCC_601:
> + *	XVYCC_709:
> + *	SYCC_601:
> + *	opYCC_601:
> + *	opRGB:
> + *	BT2020_CYCC:
> + *	DCI-P3_RGB_D65:
> + *	DCI-P3_RGB_Theater:
> + *	RGB_WIDE_FIXED:
> + *	RGB_WIDE_FLOAT:
> + *	BT601_YCC:
> + *		The behavior is undefined.
>   *
>   * Because between HDMI and DP have different colorspaces,
>   * drm_mode_create_hdmi_colorspace_property() is used for HDMI connector and
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index fe88d7fc6b8f..02c42b01a3a7 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -437,14 +437,6 @@ enum drm_privacy_screen_status {
>   *
>   * DP definitions come from the DP v2.0 spec
>   * HDMI definitions come from the CTA-861-H spec
> - *
> - * A note on YCC and RGB variants:
> - *
> - * Since userspace is not aware of the encoding on the wire
> - * (RGB or YCbCr), drivers are free to pick the appropriate
> - * variant, regardless of what userspace selects. E.g., if
> - * BT2020_RGB is selected by userspace a driver will pick
> - * BT2020_YCC if the encoding on the wire is YUV444 or YUV420.
>    *
>   * @DRM_MODE_COLORIMETRY_DEFAULT:
>   *   Driver specific behavior.

This looks really good. This also makes me need to revisit the Weston
series I've been brewing that adds "Colorspace" KMS support.

I think the two questions I had may be slightly too much in the
direction of improving rather than just documenting this property, so
I'll already give

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] drm/drm_connector: Document Colorspace property variants

[Sebastian Wick]

On Wed, Mar 06, 2024 at 10:27:21AM +0200, Pekka Paalanen wrote:
> On Tue,  5 Mar 2024 14:51:49 +0100
> Sebastian Wick <sebastian.wick@redhat.com> wrote:
> 
> > The initial idea of the Colorspace prop was that this maps 1:1 to
> > InfoFrames/SDP but KMS does not give user space enough information nor
> > control over the output format to figure out which variants can be used
> > for a given KMS commit. At the same time, properties like Broadcast RGB
> > expect full range quantization range being produced by user space from
> > the CRTC and drivers to convert to the range expected by the sink for
> > the chosen output format, mode, InfoFrames, etc.
> > 
> > This change documents the reality of the Colorspace property. The
> > Default variant unfortunately is very much driver specific and not
> > reflected by the EDID. The BT2020 variants are in active use by generic
> > compositors which have expectations from the driver about the
> > conversions it has to do when selecting certain output formats.
> > 
> > Everything else is also marked as undefined. Coming up with valid
> > behavior that makes it usable from user space and consistent with other
> > KMS properties for those variants is left as an exercise for whoever
> > wants to use them.
> > 
> > v2:
> >  * Talk about "pixel operation properties" that user space configures
> >  * Mention that user space is responsible for checking the EDID for sink
> >    support
> >  * Make it clear that drivers can choose between RGB and YCbCr on their
> >    own
> > 
> > Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
> > ---
> >  drivers/gpu/drm/drm_connector.c | 79 +++++++++++++++++++++++++--------
> >  include/drm/drm_connector.h     |  8 ----
> >  2 files changed, 61 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index b0516505f7ae..65cdcc7d22db 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -2147,24 +2147,67 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
> >   * DOC: standard connector properties
> >   *
> >   * Colorspace:
> > - *     This property helps select a suitable colorspace based on the sink
> > - *     capability. Modern sink devices support wider gamut like BT2020.
> > - *     This helps switch to BT2020 mode if the BT2020 encoded video stream
> > - *     is being played by the user, same for any other colorspace. Thereby
> > - *     giving a good visual experience to users.
> > - *
> > - *     The expectation from userspace is that it should parse the EDID
> > - *     and get supported colorspaces. Use this property and switch to the
> > - *     one supported. Sink supported colorspaces should be retrieved by
> > - *     userspace from EDID and driver will not explicitly expose them.
> > - *
> > - *     Basically the expectation from userspace is:
> > - *      - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > - *        colorspace
> > - *      - Set this new property to let the sink know what it
> > - *        converted the CRTC output to.
> > - *      - This property is just to inform sink what colorspace
> > - *        source is trying to drive.
> > + *	This property is used to inform the driver about the color encoding
> > + *	user space configured the pixel operation properties to produce.
> > + *	The variants set the colorimetry, transfer characteristics, and which
> > + *	YCbCr conversion should be used when necessary.
> > + *	The transfer characteristics from HDR_OUTPUT_METADATA takes precedence
> > + *	over this property.
> > + *	User space always configures the pixel operation properties to produce
> > + *	full quantization range data (see the Broadcast RGB property).
> > + *
> > + *	Drivers inform the sink about what colorimetry, transfer
> > + *	characteristics, YCbCr conversion, and quantization range to expect
> > + *	(this can depend on the output mode, output format and other
> > + *	properties). Drivers also convert the user space provided data to what
> > + *	the sink expects.
> 
> Hi Sebastian,
> 
> should it be more explicit that drivers are allowed to do only
> RGB->YCbCr and quantization range conversions, but not TF nor gamut
> conversions?
> 
> That is, if the driver cannot pick the TF implied by "Colorspace"
> property for the sink, then it cannot pick another TF for the sink and
> silently convert. It think this should apply to all options including
> the undefined ones. Or is that too much to guess?

That's a really good point. I'll add it in the next revision.

> > + *
> > + *	User space has to check if the sink supports all of the possible
> > + *	colorimetries that the driver is allowed to pick by parsing the EDID.
> 
> All? Rather than at least one?
> 
> Is this how it has been implemented for BT2020, that userspace picked
> colorimetry and driver picked color model and quantization are
> completely independent, and drivers do not check the combination
> against EDID?

AFAIK the driver exposes all Colorspace variants that it can support in
the driver, independent of the sink. That means user space has to make
sure that the sink supports all colorimetry variants the driver can
pick.

Would be good to get a confirmation from Harry and Ville.

> If so, "all" it is. Would be good to explain this in the commit message.

Will do.

> > + *
> > + *	For historical reasons this property exposes a number of variants which
> > + *	result in undefined behavior.
> > + *
> > + *	Default:
> > + *		The behavior is driver-specific.
> > + *	BT2020_RGB:
> > + *	BT2020_YCC:
> > + *		User space configures the pixel operation properties to produce
> > + *		RGB content with Rec. ITU-R BT.2020 colorimetry, Rec.
> > + *		ITU-R BT.2020 (Table 4, RGB) transfer characteristics and full
> > + *		quantization range.
> > + *		User space can use the HDR_OUTPUT_METADATA property to set the
> > + *		transfer characteristics to PQ (Rec. ITU-R BT.2100 Table 4) or
> > + *		HLG (Rec. ITU-R BT.2100 Table 5) in which case, user space
> > + *		configures pixel operation properties to produce content with
> > + *		the respective transfer characteristics.
> > + *		User space has to make sure the sink supports Rec.
> > + *		ITU-R BT.2020 R'G'B' and Rec. ITU-R BT.2020 Y'C'BC'R
> > + *		colorimetry.
> > + *		Drivers can configure the sink to use an RGB format, tell the
> > + *		sink to expect Rec. ITU-R BT.2020 R'G'B' colorimetry and convert
> > + *		to the appropriate quantization range.
> > + *		Drivers can configure the sink to use a YCbCr format, tell the
> > + *		sink to expect Rec. ITU-R BT.2020 Y'C'BC'R colorimetry, convert
> > + *		to YCbCr using the Rec. ITU-R BT.2020 non-constant luminance
> > + *		conversion matrix and convert to the appropriate quantization
> > + *		range.
> > + *		The variants BT2020_RGB and BT2020_YCC are equivalent and the
> > + *		driver chooses between RGB and YCbCr on its own.
> > + *	SMPTE_170M_YCC:
> > + *	BT709_YCC:
> > + *	XVYCC_601:
> > + *	XVYCC_709:
> > + *	SYCC_601:
> > + *	opYCC_601:
> > + *	opRGB:
> > + *	BT2020_CYCC:
> > + *	DCI-P3_RGB_D65:
> > + *	DCI-P3_RGB_Theater:
> > + *	RGB_WIDE_FIXED:
> > + *	RGB_WIDE_FLOAT:
> > + *	BT601_YCC:
> > + *		The behavior is undefined.
> >   *
> >   * Because between HDMI and DP have different colorspaces,
> >   * drm_mode_create_hdmi_colorspace_property() is used for HDMI connector and
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index fe88d7fc6b8f..02c42b01a3a7 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -437,14 +437,6 @@ enum drm_privacy_screen_status {
> >   *
> >   * DP definitions come from the DP v2.0 spec
> >   * HDMI definitions come from the CTA-861-H spec
> > - *
> > - * A note on YCC and RGB variants:
> > - *
> > - * Since userspace is not aware of the encoding on the wire
> > - * (RGB or YCbCr), drivers are free to pick the appropriate
> > - * variant, regardless of what userspace selects. E.g., if
> > - * BT2020_RGB is selected by userspace a driver will pick
> > - * BT2020_YCC if the encoding on the wire is YUV444 or YUV420.
> >    *
> >   * @DRM_MODE_COLORIMETRY_DEFAULT:
> >   *   Driver specific behavior.
> 
> This looks really good. This also makes me need to revisit the Weston
> series I've been brewing that adds "Colorspace" KMS support.
> 
> I think the two questions I had may be slightly too much in the
> direction of improving rather than just documenting this property, so
> I'll already give
> 
> Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Thanks.

> 
> Thanks,
> pq

Re: [PATCH v2] drm/drm_connector: Document Colorspace property variants

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 9849 bytes --]

On Wed, 6 Mar 2024 17:42:09 +0100
Sebastian Wick <sebastian.wick@redhat.com> wrote:

> On Wed, Mar 06, 2024 at 10:27:21AM +0200, Pekka Paalanen wrote:
> > On Tue,  5 Mar 2024 14:51:49 +0100
> > Sebastian Wick <sebastian.wick@redhat.com> wrote:
> >   
> > > The initial idea of the Colorspace prop was that this maps 1:1 to
> > > InfoFrames/SDP but KMS does not give user space enough information nor
> > > control over the output format to figure out which variants can be used
> > > for a given KMS commit. At the same time, properties like Broadcast RGB
> > > expect full range quantization range being produced by user space from
> > > the CRTC and drivers to convert to the range expected by the sink for
> > > the chosen output format, mode, InfoFrames, etc.
> > > 
> > > This change documents the reality of the Colorspace property. The
> > > Default variant unfortunately is very much driver specific and not
> > > reflected by the EDID. The BT2020 variants are in active use by generic
> > > compositors which have expectations from the driver about the
> > > conversions it has to do when selecting certain output formats.
> > > 
> > > Everything else is also marked as undefined. Coming up with valid
> > > behavior that makes it usable from user space and consistent with other
> > > KMS properties for those variants is left as an exercise for whoever
> > > wants to use them.
> > > 
> > > v2:
> > >  * Talk about "pixel operation properties" that user space configures
> > >  * Mention that user space is responsible for checking the EDID for sink
> > >    support
> > >  * Make it clear that drivers can choose between RGB and YCbCr on their
> > >    own
> > > 
> > > Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
> > > ---
> > >  drivers/gpu/drm/drm_connector.c | 79 +++++++++++++++++++++++++--------
> > >  include/drm/drm_connector.h     |  8 ----
> > >  2 files changed, 61 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index b0516505f7ae..65cdcc7d22db 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -2147,24 +2147,67 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
> > >   * DOC: standard connector properties
> > >   *
> > >   * Colorspace:
> > > - *     This property helps select a suitable colorspace based on the sink
> > > - *     capability. Modern sink devices support wider gamut like BT2020.
> > > - *     This helps switch to BT2020 mode if the BT2020 encoded video stream
> > > - *     is being played by the user, same for any other colorspace. Thereby
> > > - *     giving a good visual experience to users.
> > > - *
> > > - *     The expectation from userspace is that it should parse the EDID
> > > - *     and get supported colorspaces. Use this property and switch to the
> > > - *     one supported. Sink supported colorspaces should be retrieved by
> > > - *     userspace from EDID and driver will not explicitly expose them.
> > > - *
> > > - *     Basically the expectation from userspace is:
> > > - *      - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > > - *        colorspace
> > > - *      - Set this new property to let the sink know what it
> > > - *        converted the CRTC output to.
> > > - *      - This property is just to inform sink what colorspace
> > > - *        source is trying to drive.
> > > + *	This property is used to inform the driver about the color encoding
> > > + *	user space configured the pixel operation properties to produce.
> > > + *	The variants set the colorimetry, transfer characteristics, and which
> > > + *	YCbCr conversion should be used when necessary.
> > > + *	The transfer characteristics from HDR_OUTPUT_METADATA takes precedence
> > > + *	over this property.
> > > + *	User space always configures the pixel operation properties to produce
> > > + *	full quantization range data (see the Broadcast RGB property).
> > > + *
> > > + *	Drivers inform the sink about what colorimetry, transfer
> > > + *	characteristics, YCbCr conversion, and quantization range to expect
> > > + *	(this can depend on the output mode, output format and other
> > > + *	properties). Drivers also convert the user space provided data to what
> > > + *	the sink expects.  
> > 
> > Hi Sebastian,
> > 
> > should it be more explicit that drivers are allowed to do only
> > RGB->YCbCr and quantization range conversions, but not TF nor gamut
> > conversions?
> > 
> > That is, if the driver cannot pick the TF implied by "Colorspace"
> > property for the sink, then it cannot pick another TF for the sink and
> > silently convert. It think this should apply to all options including
> > the undefined ones. Or is that too much to guess?  
> 
> That's a really good point. I'll add it in the next revision.
> 
> > > + *
> > > + *	User space has to check if the sink supports all of the possible
> > > + *	colorimetries that the driver is allowed to pick by parsing the EDID.  
> > 
> > All? Rather than at least one?
> > 
> > Is this how it has been implemented for BT2020, that userspace picked
> > colorimetry and driver picked color model and quantization are
> > completely independent, and drivers do not check the combination
> > against EDID?  
> 
> AFAIK the driver exposes all Colorspace variants that it can support in
> the driver, independent of the sink. That means user space has to make
> sure that the sink supports all colorimetry variants the driver can
> pick.

I didn't mean exposing but the driver could reject the atomic commit
that would lead to a combination not advertised as supported in EDID.
If drivers reject, then userspace does not need to check for all
driver-choosable variants, just one would be enough. Theoretically not
needing all might allow some cases to work that don't support all.
"Colorspace" property value could direct the driver's choice based on
what EDID claims to support.

Of course, if drivers don't do that already, then "all" it must be.


Thanks,
pq

> Would be good to get a confirmation from Harry and Ville.
> 
> > If so, "all" it is. Would be good to explain this in the commit message.  
> 
> Will do.
> 
> > > + *
> > > + *	For historical reasons this property exposes a number of variants which
> > > + *	result in undefined behavior.
> > > + *
> > > + *	Default:
> > > + *		The behavior is driver-specific.
> > > + *	BT2020_RGB:
> > > + *	BT2020_YCC:
> > > + *		User space configures the pixel operation properties to produce
> > > + *		RGB content with Rec. ITU-R BT.2020 colorimetry, Rec.
> > > + *		ITU-R BT.2020 (Table 4, RGB) transfer characteristics and full
> > > + *		quantization range.
> > > + *		User space can use the HDR_OUTPUT_METADATA property to set the
> > > + *		transfer characteristics to PQ (Rec. ITU-R BT.2100 Table 4) or
> > > + *		HLG (Rec. ITU-R BT.2100 Table 5) in which case, user space
> > > + *		configures pixel operation properties to produce content with
> > > + *		the respective transfer characteristics.
> > > + *		User space has to make sure the sink supports Rec.
> > > + *		ITU-R BT.2020 R'G'B' and Rec. ITU-R BT.2020 Y'C'BC'R
> > > + *		colorimetry.
> > > + *		Drivers can configure the sink to use an RGB format, tell the
> > > + *		sink to expect Rec. ITU-R BT.2020 R'G'B' colorimetry and convert
> > > + *		to the appropriate quantization range.
> > > + *		Drivers can configure the sink to use a YCbCr format, tell the
> > > + *		sink to expect Rec. ITU-R BT.2020 Y'C'BC'R colorimetry, convert
> > > + *		to YCbCr using the Rec. ITU-R BT.2020 non-constant luminance
> > > + *		conversion matrix and convert to the appropriate quantization
> > > + *		range.
> > > + *		The variants BT2020_RGB and BT2020_YCC are equivalent and the
> > > + *		driver chooses between RGB and YCbCr on its own.
> > > + *	SMPTE_170M_YCC:
> > > + *	BT709_YCC:
> > > + *	XVYCC_601:
> > > + *	XVYCC_709:
> > > + *	SYCC_601:
> > > + *	opYCC_601:
> > > + *	opRGB:
> > > + *	BT2020_CYCC:
> > > + *	DCI-P3_RGB_D65:
> > > + *	DCI-P3_RGB_Theater:
> > > + *	RGB_WIDE_FIXED:
> > > + *	RGB_WIDE_FLOAT:
> > > + *	BT601_YCC:
> > > + *		The behavior is undefined.
> > >   *
> > >   * Because between HDMI and DP have different colorspaces,
> > >   * drm_mode_create_hdmi_colorspace_property() is used for HDMI connector and
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index fe88d7fc6b8f..02c42b01a3a7 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -437,14 +437,6 @@ enum drm_privacy_screen_status {
> > >   *
> > >   * DP definitions come from the DP v2.0 spec
> > >   * HDMI definitions come from the CTA-861-H spec
> > > - *
> > > - * A note on YCC and RGB variants:
> > > - *
> > > - * Since userspace is not aware of the encoding on the wire
> > > - * (RGB or YCbCr), drivers are free to pick the appropriate
> > > - * variant, regardless of what userspace selects. E.g., if
> > > - * BT2020_RGB is selected by userspace a driver will pick
> > > - * BT2020_YCC if the encoding on the wire is YUV444 or YUV420.
> > >    *
> > >   * @DRM_MODE_COLORIMETRY_DEFAULT:
> > >   *   Driver specific behavior.  
> > 
> > This looks really good. This also makes me need to revisit the Weston
> > series I've been brewing that adds "Colorspace" KMS support.
> > 
> > I think the two questions I had may be slightly too much in the
> > direction of improving rather than just documenting this property, so
> > I'll already give
> > 
> > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>  
> 
> Thanks.
> 
> > 
> > Thanks,
> > pq  
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] drm/drm_connector: Document Colorspace property variants

[Sebastian Wick]

On Thu, Mar 07, 2024 at 10:29:22AM +0200, Pekka Paalanen wrote:
> On Wed, 6 Mar 2024 17:42:09 +0100
> Sebastian Wick <sebastian.wick@redhat.com> wrote:
> 
> > On Wed, Mar 06, 2024 at 10:27:21AM +0200, Pekka Paalanen wrote:
> > > On Tue,  5 Mar 2024 14:51:49 +0100
> > > Sebastian Wick <sebastian.wick@redhat.com> wrote:
> > >   
> > > > The initial idea of the Colorspace prop was that this maps 1:1 to
> > > > InfoFrames/SDP but KMS does not give user space enough information nor
> > > > control over the output format to figure out which variants can be used
> > > > for a given KMS commit. At the same time, properties like Broadcast RGB
> > > > expect full range quantization range being produced by user space from
> > > > the CRTC and drivers to convert to the range expected by the sink for
> > > > the chosen output format, mode, InfoFrames, etc.
> > > > 
> > > > This change documents the reality of the Colorspace property. The
> > > > Default variant unfortunately is very much driver specific and not
> > > > reflected by the EDID. The BT2020 variants are in active use by generic
> > > > compositors which have expectations from the driver about the
> > > > conversions it has to do when selecting certain output formats.
> > > > 
> > > > Everything else is also marked as undefined. Coming up with valid
> > > > behavior that makes it usable from user space and consistent with other
> > > > KMS properties for those variants is left as an exercise for whoever
> > > > wants to use them.
> > > > 
> > > > v2:
> > > >  * Talk about "pixel operation properties" that user space configures
> > > >  * Mention that user space is responsible for checking the EDID for sink
> > > >    support
> > > >  * Make it clear that drivers can choose between RGB and YCbCr on their
> > > >    own
> > > > 
> > > > Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_connector.c | 79 +++++++++++++++++++++++++--------
> > > >  include/drm/drm_connector.h     |  8 ----
> > > >  2 files changed, 61 insertions(+), 26 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > index b0516505f7ae..65cdcc7d22db 100644
> > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > @@ -2147,24 +2147,67 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
> > > >   * DOC: standard connector properties
> > > >   *
> > > >   * Colorspace:
> > > > - *     This property helps select a suitable colorspace based on the sink
> > > > - *     capability. Modern sink devices support wider gamut like BT2020.
> > > > - *     This helps switch to BT2020 mode if the BT2020 encoded video stream
> > > > - *     is being played by the user, same for any other colorspace. Thereby
> > > > - *     giving a good visual experience to users.
> > > > - *
> > > > - *     The expectation from userspace is that it should parse the EDID
> > > > - *     and get supported colorspaces. Use this property and switch to the
> > > > - *     one supported. Sink supported colorspaces should be retrieved by
> > > > - *     userspace from EDID and driver will not explicitly expose them.
> > > > - *
> > > > - *     Basically the expectation from userspace is:
> > > > - *      - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > > > - *        colorspace
> > > > - *      - Set this new property to let the sink know what it
> > > > - *        converted the CRTC output to.
> > > > - *      - This property is just to inform sink what colorspace
> > > > - *        source is trying to drive.
> > > > + *	This property is used to inform the driver about the color encoding
> > > > + *	user space configured the pixel operation properties to produce.
> > > > + *	The variants set the colorimetry, transfer characteristics, and which
> > > > + *	YCbCr conversion should be used when necessary.
> > > > + *	The transfer characteristics from HDR_OUTPUT_METADATA takes precedence
> > > > + *	over this property.
> > > > + *	User space always configures the pixel operation properties to produce
> > > > + *	full quantization range data (see the Broadcast RGB property).
> > > > + *
> > > > + *	Drivers inform the sink about what colorimetry, transfer
> > > > + *	characteristics, YCbCr conversion, and quantization range to expect
> > > > + *	(this can depend on the output mode, output format and other
> > > > + *	properties). Drivers also convert the user space provided data to what
> > > > + *	the sink expects.  
> > > 
> > > Hi Sebastian,
> > > 
> > > should it be more explicit that drivers are allowed to do only
> > > RGB->YCbCr and quantization range conversions, but not TF nor gamut
> > > conversions?
> > > 
> > > That is, if the driver cannot pick the TF implied by "Colorspace"
> > > property for the sink, then it cannot pick another TF for the sink and
> > > silently convert. It think this should apply to all options including
> > > the undefined ones. Or is that too much to guess?  
> > 
> > That's a really good point. I'll add it in the next revision.
> > 
> > > > + *
> > > > + *	User space has to check if the sink supports all of the possible
> > > > + *	colorimetries that the driver is allowed to pick by parsing the EDID.  
> > > 
> > > All? Rather than at least one?
> > > 
> > > Is this how it has been implemented for BT2020, that userspace picked
> > > colorimetry and driver picked color model and quantization are
> > > completely independent, and drivers do not check the combination
> > > against EDID?  
> > 
> > AFAIK the driver exposes all Colorspace variants that it can support in
> > the driver, independent of the sink. That means user space has to make
> > sure that the sink supports all colorimetry variants the driver can
> > pick.
> 
> I didn't mean exposing but the driver could reject the atomic commit
> that would lead to a combination not advertised as supported in EDID.
> If drivers reject, then userspace does not need to check for all
> driver-choosable variants, just one would be enough. Theoretically not
> needing all might allow some cases to work that don't support all.
> "Colorspace" property value could direct the driver's choice based on
> what EDID claims to support.

Right, this could be possible and is probably even better than what I
wrote down but...

> Of course, if drivers don't do that already, then "all" it must be.

...unfortunately that seems to be the case. Maybe we can get away with
changing it though?

> 
> Thanks,
> pq
> 
> > Would be good to get a confirmation from Harry and Ville.
> > 
> > > If so, "all" it is. Would be good to explain this in the commit message.  
> > 
> > Will do.
> > 
> > > > + *
> > > > + *	For historical reasons this property exposes a number of variants which
> > > > + *	result in undefined behavior.
> > > > + *
> > > > + *	Default:
> > > > + *		The behavior is driver-specific.
> > > > + *	BT2020_RGB:
> > > > + *	BT2020_YCC:
> > > > + *		User space configures the pixel operation properties to produce
> > > > + *		RGB content with Rec. ITU-R BT.2020 colorimetry, Rec.
> > > > + *		ITU-R BT.2020 (Table 4, RGB) transfer characteristics and full
> > > > + *		quantization range.
> > > > + *		User space can use the HDR_OUTPUT_METADATA property to set the
> > > > + *		transfer characteristics to PQ (Rec. ITU-R BT.2100 Table 4) or
> > > > + *		HLG (Rec. ITU-R BT.2100 Table 5) in which case, user space
> > > > + *		configures pixel operation properties to produce content with
> > > > + *		the respective transfer characteristics.
> > > > + *		User space has to make sure the sink supports Rec.
> > > > + *		ITU-R BT.2020 R'G'B' and Rec. ITU-R BT.2020 Y'C'BC'R
> > > > + *		colorimetry.
> > > > + *		Drivers can configure the sink to use an RGB format, tell the
> > > > + *		sink to expect Rec. ITU-R BT.2020 R'G'B' colorimetry and convert
> > > > + *		to the appropriate quantization range.
> > > > + *		Drivers can configure the sink to use a YCbCr format, tell the
> > > > + *		sink to expect Rec. ITU-R BT.2020 Y'C'BC'R colorimetry, convert
> > > > + *		to YCbCr using the Rec. ITU-R BT.2020 non-constant luminance
> > > > + *		conversion matrix and convert to the appropriate quantization
> > > > + *		range.
> > > > + *		The variants BT2020_RGB and BT2020_YCC are equivalent and the
> > > > + *		driver chooses between RGB and YCbCr on its own.
> > > > + *	SMPTE_170M_YCC:
> > > > + *	BT709_YCC:
> > > > + *	XVYCC_601:
> > > > + *	XVYCC_709:
> > > > + *	SYCC_601:
> > > > + *	opYCC_601:
> > > > + *	opRGB:
> > > > + *	BT2020_CYCC:
> > > > + *	DCI-P3_RGB_D65:
> > > > + *	DCI-P3_RGB_Theater:
> > > > + *	RGB_WIDE_FIXED:
> > > > + *	RGB_WIDE_FLOAT:
> > > > + *	BT601_YCC:
> > > > + *		The behavior is undefined.
> > > >   *
> > > >   * Because between HDMI and DP have different colorspaces,
> > > >   * drm_mode_create_hdmi_colorspace_property() is used for HDMI connector and
> > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > > index fe88d7fc6b8f..02c42b01a3a7 100644
> > > > --- a/include/drm/drm_connector.h
> > > > +++ b/include/drm/drm_connector.h
> > > > @@ -437,14 +437,6 @@ enum drm_privacy_screen_status {
> > > >   *
> > > >   * DP definitions come from the DP v2.0 spec
> > > >   * HDMI definitions come from the CTA-861-H spec
> > > > - *
> > > > - * A note on YCC and RGB variants:
> > > > - *
> > > > - * Since userspace is not aware of the encoding on the wire
> > > > - * (RGB or YCbCr), drivers are free to pick the appropriate
> > > > - * variant, regardless of what userspace selects. E.g., if
> > > > - * BT2020_RGB is selected by userspace a driver will pick
> > > > - * BT2020_YCC if the encoding on the wire is YUV444 or YUV420.
> > > >    *
> > > >   * @DRM_MODE_COLORIMETRY_DEFAULT:
> > > >   *   Driver specific behavior.  
> > > 
> > > This looks really good. This also makes me need to revisit the Weston
> > > series I've been brewing that adds "Colorspace" KMS support.
> > > 
> > > I think the two questions I had may be slightly too much in the
> > > direction of improving rather than just documenting this property, so
> > > I'll already give
> > > 
> > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>  
> > 
> > Thanks.
> > 
> > > 
> > > Thanks,
> > > pq  
> > 
> > 
> 

Re: [PATCH v2] drm/drm_connector: Document Colorspace property variants

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 11157 bytes --]

On Mon, 11 Mar 2024 17:06:34 +0100
Sebastian Wick <sebastian.wick@redhat.com> wrote:

> On Thu, Mar 07, 2024 at 10:29:22AM +0200, Pekka Paalanen wrote:
> > On Wed, 6 Mar 2024 17:42:09 +0100
> > Sebastian Wick <sebastian.wick@redhat.com> wrote:
> >   
> > > On Wed, Mar 06, 2024 at 10:27:21AM +0200, Pekka Paalanen wrote:  
> > > > On Tue,  5 Mar 2024 14:51:49 +0100
> > > > Sebastian Wick <sebastian.wick@redhat.com> wrote:
> > > >     
> > > > > The initial idea of the Colorspace prop was that this maps 1:1 to
> > > > > InfoFrames/SDP but KMS does not give user space enough information nor
> > > > > control over the output format to figure out which variants can be used
> > > > > for a given KMS commit. At the same time, properties like Broadcast RGB
> > > > > expect full range quantization range being produced by user space from
> > > > > the CRTC and drivers to convert to the range expected by the sink for
> > > > > the chosen output format, mode, InfoFrames, etc.
> > > > > 
> > > > > This change documents the reality of the Colorspace property. The
> > > > > Default variant unfortunately is very much driver specific and not
> > > > > reflected by the EDID. The BT2020 variants are in active use by generic
> > > > > compositors which have expectations from the driver about the
> > > > > conversions it has to do when selecting certain output formats.
> > > > > 
> > > > > Everything else is also marked as undefined. Coming up with valid
> > > > > behavior that makes it usable from user space and consistent with other
> > > > > KMS properties for those variants is left as an exercise for whoever
> > > > > wants to use them.
> > > > > 
> > > > > v2:
> > > > >  * Talk about "pixel operation properties" that user space configures
> > > > >  * Mention that user space is responsible for checking the EDID for sink
> > > > >    support
> > > > >  * Make it clear that drivers can choose between RGB and YCbCr on their
> > > > >    own
> > > > > 
> > > > > Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_connector.c | 79 +++++++++++++++++++++++++--------
> > > > >  include/drm/drm_connector.h     |  8 ----
> > > > >  2 files changed, 61 insertions(+), 26 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > > index b0516505f7ae..65cdcc7d22db 100644
> > > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > > @@ -2147,24 +2147,67 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
> > > > >   * DOC: standard connector properties
> > > > >   *
> > > > >   * Colorspace:
> > > > > - *     This property helps select a suitable colorspace based on the sink
> > > > > - *     capability. Modern sink devices support wider gamut like BT2020.
> > > > > - *     This helps switch to BT2020 mode if the BT2020 encoded video stream
> > > > > - *     is being played by the user, same for any other colorspace. Thereby
> > > > > - *     giving a good visual experience to users.
> > > > > - *
> > > > > - *     The expectation from userspace is that it should parse the EDID
> > > > > - *     and get supported colorspaces. Use this property and switch to the
> > > > > - *     one supported. Sink supported colorspaces should be retrieved by
> > > > > - *     userspace from EDID and driver will not explicitly expose them.
> > > > > - *
> > > > > - *     Basically the expectation from userspace is:
> > > > > - *      - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > > > > - *        colorspace
> > > > > - *      - Set this new property to let the sink know what it
> > > > > - *        converted the CRTC output to.
> > > > > - *      - This property is just to inform sink what colorspace
> > > > > - *        source is trying to drive.
> > > > > + *	This property is used to inform the driver about the color encoding
> > > > > + *	user space configured the pixel operation properties to produce.
> > > > > + *	The variants set the colorimetry, transfer characteristics, and which
> > > > > + *	YCbCr conversion should be used when necessary.
> > > > > + *	The transfer characteristics from HDR_OUTPUT_METADATA takes precedence
> > > > > + *	over this property.
> > > > > + *	User space always configures the pixel operation properties to produce
> > > > > + *	full quantization range data (see the Broadcast RGB property).
> > > > > + *
> > > > > + *	Drivers inform the sink about what colorimetry, transfer
> > > > > + *	characteristics, YCbCr conversion, and quantization range to expect
> > > > > + *	(this can depend on the output mode, output format and other
> > > > > + *	properties). Drivers also convert the user space provided data to what
> > > > > + *	the sink expects.    
> > > > 
> > > > Hi Sebastian,
> > > > 
> > > > should it be more explicit that drivers are allowed to do only
> > > > RGB->YCbCr and quantization range conversions, but not TF nor gamut
> > > > conversions?
> > > > 
> > > > That is, if the driver cannot pick the TF implied by "Colorspace"
> > > > property for the sink, then it cannot pick another TF for the sink and
> > > > silently convert. It think this should apply to all options including
> > > > the undefined ones. Or is that too much to guess?    
> > > 
> > > That's a really good point. I'll add it in the next revision.
> > >   
> > > > > + *
> > > > > + *	User space has to check if the sink supports all of the possible
> > > > > + *	colorimetries that the driver is allowed to pick by parsing the EDID.    
> > > > 
> > > > All? Rather than at least one?
> > > > 
> > > > Is this how it has been implemented for BT2020, that userspace picked
> > > > colorimetry and driver picked color model and quantization are
> > > > completely independent, and drivers do not check the combination
> > > > against EDID?    
> > > 
> > > AFAIK the driver exposes all Colorspace variants that it can support in
> > > the driver, independent of the sink. That means user space has to make
> > > sure that the sink supports all colorimetry variants the driver can
> > > pick.  
> > 
> > I didn't mean exposing but the driver could reject the atomic commit
> > that would lead to a combination not advertised as supported in EDID.
> > If drivers reject, then userspace does not need to check for all
> > driver-choosable variants, just one would be enough. Theoretically not
> > needing all might allow some cases to work that don't support all.
> > "Colorspace" property value could direct the driver's choice based on
> > what EDID claims to support.  
> 
> Right, this could be possible and is probably even better than what I
> wrote down but...
> 
> > Of course, if drivers don't do that already, then "all" it must be.  
> 
> ...unfortunately that seems to be the case. Maybe we can get away with
> changing it though?

I wouldn't risk it at this point, I think :-)

It's another battle.


Thanks,
pq

> > > Would be good to get a confirmation from Harry and Ville.
> > >   
> > > > If so, "all" it is. Would be good to explain this in the commit message.    
> > > 
> > > Will do.
> > >   
> > > > > + *
> > > > > + *	For historical reasons this property exposes a number of variants which
> > > > > + *	result in undefined behavior.
> > > > > + *
> > > > > + *	Default:
> > > > > + *		The behavior is driver-specific.
> > > > > + *	BT2020_RGB:
> > > > > + *	BT2020_YCC:
> > > > > + *		User space configures the pixel operation properties to produce
> > > > > + *		RGB content with Rec. ITU-R BT.2020 colorimetry, Rec.
> > > > > + *		ITU-R BT.2020 (Table 4, RGB) transfer characteristics and full
> > > > > + *		quantization range.
> > > > > + *		User space can use the HDR_OUTPUT_METADATA property to set the
> > > > > + *		transfer characteristics to PQ (Rec. ITU-R BT.2100 Table 4) or
> > > > > + *		HLG (Rec. ITU-R BT.2100 Table 5) in which case, user space
> > > > > + *		configures pixel operation properties to produce content with
> > > > > + *		the respective transfer characteristics.
> > > > > + *		User space has to make sure the sink supports Rec.
> > > > > + *		ITU-R BT.2020 R'G'B' and Rec. ITU-R BT.2020 Y'C'BC'R
> > > > > + *		colorimetry.
> > > > > + *		Drivers can configure the sink to use an RGB format, tell the
> > > > > + *		sink to expect Rec. ITU-R BT.2020 R'G'B' colorimetry and convert
> > > > > + *		to the appropriate quantization range.
> > > > > + *		Drivers can configure the sink to use a YCbCr format, tell the
> > > > > + *		sink to expect Rec. ITU-R BT.2020 Y'C'BC'R colorimetry, convert
> > > > > + *		to YCbCr using the Rec. ITU-R BT.2020 non-constant luminance
> > > > > + *		conversion matrix and convert to the appropriate quantization
> > > > > + *		range.
> > > > > + *		The variants BT2020_RGB and BT2020_YCC are equivalent and the
> > > > > + *		driver chooses between RGB and YCbCr on its own.
> > > > > + *	SMPTE_170M_YCC:
> > > > > + *	BT709_YCC:
> > > > > + *	XVYCC_601:
> > > > > + *	XVYCC_709:
> > > > > + *	SYCC_601:
> > > > > + *	opYCC_601:
> > > > > + *	opRGB:
> > > > > + *	BT2020_CYCC:
> > > > > + *	DCI-P3_RGB_D65:
> > > > > + *	DCI-P3_RGB_Theater:
> > > > > + *	RGB_WIDE_FIXED:
> > > > > + *	RGB_WIDE_FLOAT:
> > > > > + *	BT601_YCC:
> > > > > + *		The behavior is undefined.
> > > > >   *
> > > > >   * Because between HDMI and DP have different colorspaces,
> > > > >   * drm_mode_create_hdmi_colorspace_property() is used for HDMI connector and
> > > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > > > index fe88d7fc6b8f..02c42b01a3a7 100644
> > > > > --- a/include/drm/drm_connector.h
> > > > > +++ b/include/drm/drm_connector.h
> > > > > @@ -437,14 +437,6 @@ enum drm_privacy_screen_status {
> > > > >   *
> > > > >   * DP definitions come from the DP v2.0 spec
> > > > >   * HDMI definitions come from the CTA-861-H spec
> > > > > - *
> > > > > - * A note on YCC and RGB variants:
> > > > > - *
> > > > > - * Since userspace is not aware of the encoding on the wire
> > > > > - * (RGB or YCbCr), drivers are free to pick the appropriate
> > > > > - * variant, regardless of what userspace selects. E.g., if
> > > > > - * BT2020_RGB is selected by userspace a driver will pick
> > > > > - * BT2020_YCC if the encoding on the wire is YUV444 or YUV420.
> > > > >    *
> > > > >   * @DRM_MODE_COLORIMETRY_DEFAULT:
> > > > >   *   Driver specific behavior.    
> > > > 
> > > > This looks really good. This also makes me need to revisit the Weston
> > > > series I've been brewing that adds "Colorspace" KMS support.
> > > > 
> > > > I think the two questions I had may be slightly too much in the
> > > > direction of improving rather than just documenting this property, so
> > > > I'll already give
> > > > 
> > > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>    
> > > 
> > > Thanks.
> > >   
> > > > 
> > > > Thanks,
> > > > pq    
> > > 
> > >   
> >   
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]