On Fri,  3 Feb 2023 02:07:44 +0000
Joshua Ashton <jos...@froggi.es> wrote:

> 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.
> 
> Note: _CYCC was effectively 'removed' by this change, but that was not
> possible to be taken advantage of anyway, as there is currently no
> pixel_encoding control so it would not be possible to output
> linear YCbCr.
> 
> Signed-off-by: Joshua Ashton <jos...@froggi.es>
> 
> Cc: Pekka Paalanen <ppaala...@gmail.com>
> Cc: Sebastian Wick <sebastian.w...@redhat.com>
> Cc: vitaly.pros...@amd.com
> Cc: Uma Shankar <uma.shan...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Cc: Joshua Ashton <jos...@froggi.es>
> Cc: dri-de...@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> ---
>  drivers/gpu/drm/display/drm_hdmi_helper.c |  9 ++++-----
>  drivers/gpu/drm/drm_connector.c           | 12 ++++++------
>  drivers/gpu/drm/i915/display/intel_dp.c   | 20 +++++++++-----------
>  include/drm/drm_connector.h               | 19 ++++++++++---------
>  4 files changed, 29 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c 
> b/drivers/gpu/drm/display/drm_hdmi_helper.c
> index 0264abe55278..c85860600395 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c
> @@ -99,8 +99,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))
>  
> @@ -113,9 +112,9 @@ static const u32 hdmi_colorimetry_val[] = {
>       [DRM_MODE_COLORIMETRY_SYCC_601] = HDMI_COLORIMETRY_SYCC_601,
>       [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_1] = HDMI_COLORIMETRY_BT2020,
> +     [DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_2] = 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..58699ab15a6a 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1029,11 +1029,11 @@ static const struct drm_prop_enum_list 
> hdmi_colorspaces[] = {
>       /* Colorimetry based on IEC 61966-2-5 */
>       { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" },
>       /* Colorimetry based on ITU-R BT.2020 */
> -     { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> +     { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_1, "BT2020_DEPRECATED_1" },
>       /* Colorimetry based on ITU-R BT.2020 */
> -     { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
> +     { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_2, "BT2020_DEPRECATED_2" },
>       /* Colorimetry based on ITU-R BT.2020 */
> -     { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
> +     { DRM_MODE_COLORIMETRY_BT2020, "BT2020" },
>       /* 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 */
> @@ -1066,9 +1066,9 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
> = {
>       /* Colorimetry based on IEC 61966-2-5 [33] */
>       { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" },
>       /* Colorimetry based on ITU-R BT.2020 */
> -     { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> +     { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_1, "BT2020_DEPRECATED_1" },
>       /* Colorimetry based on ITU-R BT.2020 */
> -     { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
> +     { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_2, "BT2020_DEPRECATED_2" },
>  };

Hi,

do these not rename the old uAPI strings?

Shouldn't the old strings be kept? It's much easier to scream "kernel
regression" when the expected string is no longer found than a subtle
change in behaviour that might not even be a change. ;-)

If there is not going to be a difference in behaviour, the enum could
expose e.g. all of "BT2020_RGB", "BT2020_CYCC" and "BT2020_YCC" as the
same integer value. If old userspace exists, it would not notice any
difference.

I mean, the *strings* are the uAPI, not the integers, right?


Thanks,
pq

>  
>  /**
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index c9be61d2348e..1aa5dedeec7b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1763,14 +1763,12 @@ static void intel_dp_compute_vsc_colorimetry(const 
> struct intel_crtc_state *crtc
>       case DRM_MODE_COLORIMETRY_OPYCC_601:
>               vsc->colorimetry = DP_COLORIMETRY_OPYCC_601;
>               break;
> -     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_1:
> +     case DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_2:
> +     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 +3041,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_1:
> +     case DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_2:
> +     case DRM_MODE_COLORIMETRY_BT2020:
>               return true;
>       default:
>               break;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index eb4cc9076e16..42a3cf43168c 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -390,12 +390,13 @@ enum drm_privacy_screen_status {
>   *   opYCC601 colorimetry format
>   * @DRM_MODE_COLORIMETRY_OPRGB:
>   *   opRGB colorimetry format
> - * @DRM_MODE_COLORIMETRY_BT2020_CYCC:
> - *   ITU-R BT.2020 Y'c C'bc C'rc (linear) colorimetry format
> - * @DRM_MODE_COLORIMETRY_BT2020_RGB:
> - *   ITU-R BT.2020 R' G' B' colorimetry format
> - * @DRM_MODE_COLORIMETRY_BT2020_YCC:
> - *   ITU-R BT.2020 Y' C'b C'r colorimetry format
> + * @DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_1:
> + * @DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_2:
> + * @DRM_MODE_COLORIMETRY_BT2020:
> + *   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)
> + *   colorimetry format
>   * @DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
>   *   DCI-P3 (SMPTE RP 431-2) colorimetry format
>   * @DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER:
> @@ -420,9 +421,9 @@ enum drm_colorspace {
>       DRM_MODE_COLORIMETRY_SYCC_601,
>       DRM_MODE_COLORIMETRY_OPYCC_601,
>       DRM_MODE_COLORIMETRY_OPRGB,
> -     DRM_MODE_COLORIMETRY_BT2020_CYCC,
> -     DRM_MODE_COLORIMETRY_BT2020_RGB,
> -     DRM_MODE_COLORIMETRY_BT2020_YCC,
> +     DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_1,
> +     DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_2,
> +     DRM_MODE_COLORIMETRY_BT2020,
>       /* Additional Colorimetry extension added as part of CTA 861.G */
>       DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65,
>       DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER,

Attachment: pgp7DuMPS6kx3.pgp
Description: OpenPGP digital signature

Reply via email to