On Tue, 7 Mar 2023 10:10:53 -0500
Harry Wentland <harry.wentland@xxxxxxx> wrote:
> From: Joshua Ashton <joshua@xxxxxxxxx>
>
> Userspace has no way of controlling or knowing the pixel encoding
> currently, so there is no way for it to ever get the right values here.
>
> When we do add pixel_encoding control from userspace,we can pick the
> right value for the colorimetry packet based on the
> pixel_encoding + the colorspace.
>
> Let's deprecate these values, and have one BT.2020 colorspace entry
> that userspace can use.
>
> v2:
> - leave CYCC alone for now; it serves a purpose
> - leave BT2020_RGB the new default BT2020
>
> Signed-off-by: Joshua Ashton <joshua@xxxxxxxxx>
> Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx>
> Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx>
>
> Cc: Pekka Paalanen <ppaalanen@xxxxxxxxx>
> Cc: Sebastian Wick <sebastian.wick@xxxxxxxxxx>
> Cc: Vitaly.Prosyak@xxxxxxx
> Cc: Uma Shankar <uma.shankar@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Joshua Ashton <joshua@xxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> ---
> drivers/gpu/drm/display/drm_hdmi_helper.c | 7 +++----
> drivers/gpu/drm/drm_connector.c | 8 ++++----
> drivers/gpu/drm/i915/display/intel_dp.c | 14 +++++++-------
> include/drm/drm_connector.h | 15 +++++++++------
> 4 files changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c b/drivers/gpu/drm/display/drm_hdmi_helper.c
> index faf5e9efa7d3..05a0d03ffcda 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c
> @@ -97,8 +97,7 @@ EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
> #define HDMI_COLORIMETRY_OPYCC_601 (C(3) | EC(3) | ACE(0))
> #define HDMI_COLORIMETRY_OPRGB (C(3) | EC(4) | ACE(0))
> #define HDMI_COLORIMETRY_BT2020_CYCC (C(3) | EC(5) | ACE(0))
> -#define HDMI_COLORIMETRY_BT2020_RGB (C(3) | EC(6) | ACE(0))
> -#define HDMI_COLORIMETRY_BT2020_YCC (C(3) | EC(6) | ACE(0))
> +#define HDMI_COLORIMETRY_BT2020 (C(3) | EC(6) | ACE(0))
> #define HDMI_COLORIMETRY_DCI_P3_RGB_D65 (C(3) | EC(7) | ACE(0))
> #define HDMI_COLORIMETRY_DCI_P3_RGB_THEATER (C(3) | EC(7) | ACE(1))
>
> @@ -112,8 +111,8 @@ static const u32 hdmi_colorimetry_val[] = {
> [DRM_MODE_COLORIMETRY_OPYCC_601] = HDMI_COLORIMETRY_OPYCC_601,
> [DRM_MODE_COLORIMETRY_OPRGB] = HDMI_COLORIMETRY_OPRGB,
> [DRM_MODE_COLORIMETRY_BT2020_CYCC] = HDMI_COLORIMETRY_BT2020_CYCC,
> - [DRM_MODE_COLORIMETRY_BT2020_RGB] = HDMI_COLORIMETRY_BT2020_RGB,
> - [DRM_MODE_COLORIMETRY_BT2020_YCC] = HDMI_COLORIMETRY_BT2020_YCC,
> + [DRM_MODE_COLORIMETRY_BT2020_DEPRECATED] = HDMI_COLORIMETRY_BT2020,
> + [DRM_MODE_COLORIMETRY_BT2020] = HDMI_COLORIMETRY_BT2020,
> };
>
> #undef C
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 61c29ce74b03..fe7eab15f727 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1031,9 +1031,9 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> /* Colorimetry based on ITU-R BT.2020 */
> { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> /* Colorimetry based on ITU-R BT.2020 */
> - { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
> + { DRM_MODE_COLORIMETRY_BT2020, "BT2020" },
> /* Colorimetry based on ITU-R BT.2020 */
> - { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
> + { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED, "BT2020_DEPRECATED" },
> /* Added as part of Additional Colorimetry Extension in 861.G */
> { DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65, "DCI-P3_RGB_D65" },
> { DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
> @@ -1054,7 +1054,7 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
> /* Colorimetry based on SMPTE RP 431-2 */
> { DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65, "DCI-P3_RGB_D65" },
> /* Colorimetry based on ITU-R BT.2020 */
> - { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
> + { DRM_MODE_COLORIMETRY_BT2020, "BT2020" },
> { DRM_MODE_COLORIMETRY_BT601_YCC, "BT601_YCC" },
> { DRM_MODE_COLORIMETRY_BT709_YCC, "BT709_YCC" },
> /* Standard Definition Colorimetry based on IEC 61966-2-4 */
> @@ -1068,7 +1068,7 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
> /* Colorimetry based on ITU-R BT.2020 */
> { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> /* Colorimetry based on ITU-R BT.2020 */
> - { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
> + { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED, "BT2020_DEPRECATED" },
Let's hope no-one complains about missing the old string names in UABI. :-)
Actually, you should write in the commit message why removing old names
is fine.
> };
>
> /**
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index c9be61d2348e..be100a193bf5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1766,11 +1766,11 @@ static void intel_dp_compute_vsc_colorimetry(const struct intel_crtc_state *crtc
> case DRM_MODE_COLORIMETRY_BT2020_CYCC:
> vsc->colorimetry = DP_COLORIMETRY_BT2020_CYCC;
> break;
> - case DRM_MODE_COLORIMETRY_BT2020_RGB:
> - vsc->colorimetry = DP_COLORIMETRY_BT2020_RGB;
> - break;
> - case DRM_MODE_COLORIMETRY_BT2020_YCC:
> - vsc->colorimetry = DP_COLORIMETRY_BT2020_YCC;
> + case DRM_MODE_COLORIMETRY_BT2020_DEPRECATED:
> + case DRM_MODE_COLORIMETRY_BT2020:
> + vsc->colorimetry = vsc->pixelformat == DP_PIXELFORMAT_RGB
> + ? DP_COLORIMETRY_BT2020_RGB
> + : DP_COLORIMETRY_BT2020_YCC;
> break;
> case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
> case DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER:
> @@ -3043,9 +3043,9 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state,
> switch (conn_state->colorspace) {
> case DRM_MODE_COLORIMETRY_SYCC_601:
> case DRM_MODE_COLORIMETRY_OPYCC_601:
> - case DRM_MODE_COLORIMETRY_BT2020_YCC:
> - case DRM_MODE_COLORIMETRY_BT2020_RGB:
> case DRM_MODE_COLORIMETRY_BT2020_CYCC:
> + case DRM_MODE_COLORIMETRY_BT2020_DEPRECATED:
> + case DRM_MODE_COLORIMETRY_BT2020:
> return true;
> default:
> break;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index bb078666dc34..3e2e1bc7aa04 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -409,12 +409,15 @@ enum drm_privacy_screen_status {
> * @DRM_MODE_COLORIMETRY_BT2020_CYCC:
> * (HDMI, DP)
> * ITU-R BT.2020 Y'c C'bc C'rc (constant luminance) colorimetry format
> - * @DRM_MODE_COLORIMETRY_BT2020_RGB:
> + * @DRM_MODE_COLORIMETRY_BT2020:
> * (HDMI, DP)
> - * ITU-R BT.2020 R' G' B' colorimetry format
> - * @DRM_MODE_COLORIMETRY_BT2020_YCC:
> + * ITU-R BT.2020 [R' G' B'] or
> + * ITU-R BT.2020 [Y' C'b C'r] or
> + * ITU-R BT.2020 [Y'c C'bc C'rc] (linear)
Drop the constant luminance variant from this value's doc.
> + * colorimetry format
> + * @DRM_MODE_COLORIMETRY_BT2020_DEPRECATED:
> * (HDMI, DP)
> - * ITU-R BT.2020 Y' C'b C'r colorimetry format
> + * deprecated; same as DRM_MODE_COLORIMETRY_BT2020
> * @DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
> * (HDMI)
> * SMPTE ST 2113 P3D65 colorimetry format
> @@ -448,8 +451,8 @@ enum drm_colorspace {
> DRM_MODE_COLORIMETRY_OPYCC_601 = 6,
> DRM_MODE_COLORIMETRY_OPRGB = 7,
> DRM_MODE_COLORIMETRY_BT2020_CYCC = 8,
> - DRM_MODE_COLORIMETRY_BT2020_RGB = 9,
> - DRM_MODE_COLORIMETRY_BT2020_YCC = 10,
> + DRM_MODE_COLORIMETRY_BT2020 = 9,
> + DRM_MODE_COLORIMETRY_BT2020_DEPRECATED = 10,
> /* Additional Colorimetry extension added as part of CTA 861.G */
> DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65 = 11,
> DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER = 12,
With the caveat noted and nitpick fixed:
Acked-by: Pekka Paalanen <pekka.paalanen@xxxxxxxxxxxxx>
Thanks,
pq
Attachment:
pgpXmtK6UahoC.pgp
Description: OpenPGP digital signature
