[PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
Joshua Ashton
joshua at froggi.es
Thu Feb 9 16:38:54 UTC 2023
On 2/8/23 09:30, Pekka Paalanen wrote:
> On Fri, 3 Feb 2023 02:07:44 +0000
> Joshua Ashton <joshua at 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 <joshua at froggi.es>
>>
>> Cc: Pekka Paalanen <ppaalanen at gmail.com>
>> Cc: Sebastian Wick <sebastian.wick at redhat.com>
>> Cc: Vitaly.Prosyak at amd.com
>> Cc: Uma Shankar <uma.shankar at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Joshua Ashton <joshua at froggi.es>
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: amd-gfx at 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?
Both are uAPI these days.
I was wrong when I did this commit either way.
- Joshie 🐸✨
>
>
> 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,
>
More information about the amd-gfx
mailing list