Drivers might not support all colorspaces defined in dp_colorspaces and hdmi_colorspaces. This results in undefined behavior when userspace is setting an unsupported colorspace. Allow drivers to pass the list of supported colorspaces when creating the colorspace property. v2: - Use 0 to indicate support for all colorspaces (Jani) - Print drm_dbg_kms message when drivers pass 0 to signal that drivers should specify supported colorspaecs explicity (Jani) Signed-off-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: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx Reviewed-By: Joshua Ashton <joshua@xxxxxxxxx> --- drivers/gpu/drm/drm_connector.c | 148 ++++++++++-------- .../gpu/drm/i915/display/intel_connector.c | 4 +- drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +- include/drm/drm_connector.h | 8 +- 4 files changed, 91 insertions(+), 71 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index ddba0b9fcc17..8e81105fb2ab 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1012,64 +1012,57 @@ static const struct drm_prop_enum_list drm_dp_subconnector_enum_list[] = { DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name, drm_dp_subconnector_enum_list) -static const struct drm_prop_enum_list hdmi_colorspaces[] = { - /* For Default case, driver will set the colorspace */ - { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, - /* Standard Definition Colorimetry based on CEA 861 */ - { DRM_MODE_COLORIMETRY_SMPTE_170M_YCC, "SMPTE_170M_YCC" }, - { DRM_MODE_COLORIMETRY_BT709_YCC, "BT709_YCC" }, - /* Standard Definition Colorimetry based on IEC 61966-2-4 */ - { DRM_MODE_COLORIMETRY_XVYCC_601, "XVYCC_601" }, - /* High Definition Colorimetry based on IEC 61966-2-4 */ - { DRM_MODE_COLORIMETRY_XVYCC_709, "XVYCC_709" }, - /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ - { DRM_MODE_COLORIMETRY_SYCC_601, "SYCC_601" }, - /* Colorimetry based on IEC 61966-2-5 [33] */ - { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" }, - /* 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" }, - /* Colorimetry based on ITU-R BT.2020 */ - { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" }, - /* Colorimetry based on ITU-R BT.2020 */ - { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, - /* 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" }, +static const char * const colorspace_names[] = { + [DRM_MODE_COLORIMETRY_DEFAULT] = "Default", + [DRM_MODE_COLORIMETRY_SMPTE_170M_YCC] = "SMPTE_170M_YCC", + [DRM_MODE_COLORIMETRY_BT709_YCC] = "BT709_YCC", + [DRM_MODE_COLORIMETRY_XVYCC_601] = "XVYCC_601", + [DRM_MODE_COLORIMETRY_XVYCC_709] = "XVYCC_709", + [DRM_MODE_COLORIMETRY_SYCC_601] = "SYCC_601", + [DRM_MODE_COLORIMETRY_OPYCC_601] = "opYCC_601", + [DRM_MODE_COLORIMETRY_OPRGB] = "opRGB", + [DRM_MODE_COLORIMETRY_BT2020_CYCC] = "BT2020_CYCC", + [DRM_MODE_COLORIMETRY_BT2020_RGB] = "BT2020_RGB", + [DRM_MODE_COLORIMETRY_BT2020_YCC] = "BT2020_YCC", + [DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65] = "P3_RGB_D65", + [DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER] = "P3_RGB_Theater", + [DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED] = "RGB_WIDE_FIXED", + [DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT] = "RGB_WIDE_FLOAT", + [DRM_MODE_COLORIMETRY_BT601_YCC] = "BT601_YCC", }; +static const u32 hdmi_colorspaces = + BIT(DRM_MODE_COLORIMETRY_SMPTE_170M_YCC) | + BIT(DRM_MODE_COLORIMETRY_BT709_YCC) | + BIT(DRM_MODE_COLORIMETRY_XVYCC_601) | + BIT(DRM_MODE_COLORIMETRY_XVYCC_709) | + BIT(DRM_MODE_COLORIMETRY_SYCC_601) | + BIT(DRM_MODE_COLORIMETRY_OPYCC_601) | + BIT(DRM_MODE_COLORIMETRY_OPRGB) | + BIT(DRM_MODE_COLORIMETRY_BT2020_CYCC) | + BIT(DRM_MODE_COLORIMETRY_BT2020_RGB) | + BIT(DRM_MODE_COLORIMETRY_BT2020_YCC) | + BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65) | + BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER); + /* * As per DP 1.4a spec, 2.2.5.7.5 VSC SDP Payload for Pixel Encoding/Colorimetry * Format Table 2-120 */ -static const struct drm_prop_enum_list dp_colorspaces[] = { - /* For Default case, driver will set the colorspace */ - { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, - { DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED, "RGB_Wide_Gamut_Fixed_Point" }, - /* Colorimetry based on scRGB (IEC 61966-2-2) */ - { DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT, "RGB_Wide_Gamut_Floating_Point" }, - /* Colorimetry based on IEC 61966-2-5 */ - { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, - /* 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_BT601_YCC, "BT601_YCC" }, - { DRM_MODE_COLORIMETRY_BT709_YCC, "BT709_YCC" }, - /* Standard Definition Colorimetry based on IEC 61966-2-4 */ - { DRM_MODE_COLORIMETRY_XVYCC_601, "XVYCC_601" }, - /* High Definition Colorimetry based on IEC 61966-2-4 */ - { DRM_MODE_COLORIMETRY_XVYCC_709, "XVYCC_709" }, - /* Colorimetry based on IEC 61966-2-1/Amendment 1 */ - { DRM_MODE_COLORIMETRY_SYCC_601, "SYCC_601" }, - /* 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" }, - /* Colorimetry based on ITU-R BT.2020 */ - { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" }, -}; +static const u32 dp_colorspaces = + BIT(DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED) | + BIT(DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT) | + BIT(DRM_MODE_COLORIMETRY_OPRGB) | + BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65) | + BIT(DRM_MODE_COLORIMETRY_BT2020_RGB) | + BIT(DRM_MODE_COLORIMETRY_BT601_YCC) | + BIT(DRM_MODE_COLORIMETRY_BT709_YCC) | + BIT(DRM_MODE_COLORIMETRY_XVYCC_601) | + BIT(DRM_MODE_COLORIMETRY_XVYCC_709) | + BIT(DRM_MODE_COLORIMETRY_SYCC_601) | + BIT(DRM_MODE_COLORIMETRY_OPYCC_601) | + BIT(DRM_MODE_COLORIMETRY_BT2020_CYCC) | + BIT(DRM_MODE_COLORIMETRY_BT2020_YCC); /** * DOC: standard connector properties @@ -1972,30 +1965,49 @@ EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property); */ static int drm_mode_create_colorspace_property(struct drm_connector *connector, - const struct drm_prop_enum_list *colorspaces, - int size) + u32 supported_colorspaces) { struct drm_device *dev = connector->dev; + u32 colorspaces = supported_colorspaces | BIT(DRM_MODE_COLORIMETRY_DEFAULT); + struct drm_prop_enum_list enum_list[DRM_MODE_COLORIMETRY_MAX]; + int i, len; if (connector->colorspace_property) return 0; - if (!colorspaces) - return 0; + if (!supported_colorspaces) + drm_dbg_kms(dev, "Driver is not passing supported colorspaces on [CONNECTOR:%d:%s]\n", + connector->base.id, connector->name); + + if ((supported_colorspaces & -BIT(DRM_MODE_COLORIMETRY_MAX)) != 0) + return -EINVAL; + + len = 0; + for (i = 0; i < DRM_MODE_COLORIMETRY_MAX; i++) { + if (supported_colorspaces != 0 && (colorspaces & BIT(i)) == 0) + continue; + + enum_list[len].type = i; + enum_list[len].name = colorspace_names[i]; + len++; + } connector->colorspace_property = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, "Colorspace", - colorspaces, - size); + enum_list, + len); if (!connector->colorspace_property) return -ENOMEM; return 0; } + /** * drm_mode_create_hdmi_colorspace_property - create hdmi colorspace property * @connector: connector to create the Colorspace property on. + * @supported_colorspaces: A bitfield of supported colorspaces or 0 for all + * HDMI colorspaces * * Called by a driver the first time it's needed, must be attached to desired * HDMI connectors. @@ -2003,17 +2015,20 @@ static int drm_mode_create_colorspace_property(struct drm_connector *connector, * Returns: * Zero on success, negative errno on failure. */ -int drm_mode_create_hdmi_colorspace_property(struct drm_connector *connector) +int drm_mode_create_hdmi_colorspace_property(struct drm_connector *connector, + u32 supported_colorspaces) { - return drm_mode_create_colorspace_property(connector, - hdmi_colorspaces, - ARRAY_SIZE(hdmi_colorspaces)); + u32 colorspaces = supported_colorspaces & hdmi_colorspaces; + + return drm_mode_create_colorspace_property(connector, colorspaces); } EXPORT_SYMBOL(drm_mode_create_hdmi_colorspace_property); /** * drm_mode_create_dp_colorspace_property - create dp colorspace property * @connector: connector to create the Colorspace property on. + * @supported_colorspaces: A bitfield of supported colorspaces or 0 for all + * DP colorspaces * * Called by a driver the first time it's needed, must be attached to desired * DP connectors. @@ -2021,11 +2036,12 @@ EXPORT_SYMBOL(drm_mode_create_hdmi_colorspace_property); * Returns: * Zero on success, negative errno on failure. */ -int drm_mode_create_dp_colorspace_property(struct drm_connector *connector) +int drm_mode_create_dp_colorspace_property(struct drm_connector *connector, + u32 supported_colorspaces) { - return drm_mode_create_colorspace_property(connector, - dp_colorspaces, - ARRAY_SIZE(dp_colorspaces)); + u32 colorspaces = supported_colorspaces & dp_colorspaces; + + return drm_mode_create_colorspace_property(connector, colorspaces); } EXPORT_SYMBOL(drm_mode_create_dp_colorspace_property); diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c index 6d5cbeb8df4d..9e4b054266ea 100644 --- a/drivers/gpu/drm/i915/display/intel_connector.c +++ b/drivers/gpu/drm/i915/display/intel_connector.c @@ -283,13 +283,13 @@ intel_attach_aspect_ratio_property(struct drm_connector *connector) void intel_attach_hdmi_colorspace_property(struct drm_connector *connector) { - if (!drm_mode_create_hdmi_colorspace_property(connector)) + if (!drm_mode_create_hdmi_colorspace_property(connector, 0)) drm_connector_attach_colorspace_property(connector); } void intel_attach_dp_colorspace_property(struct drm_connector *connector) { - if (!drm_mode_create_dp_colorspace_property(connector)) + if (!drm_mode_create_dp_colorspace_property(connector, 0)) drm_connector_attach_colorspace_property(connector); } diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 9e145690c480..95d73b817b05 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -605,7 +605,7 @@ static int vc4_hdmi_connector_init(struct drm_device *dev, if (ret) return ret; - ret = drm_mode_create_hdmi_colorspace_property(connector); + ret = drm_mode_create_hdmi_colorspace_property(connector, 0); if (ret) return ret; diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index edef65388c29..5825c6ab969b 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -30,6 +30,7 @@ #include <linux/notifier.h> #include <drm/drm_mode_object.h> #include <drm/drm_util.h> +#include <drm/drm_property.h> #include <uapi/drm/drm_mode.h> @@ -393,6 +394,7 @@ enum drm_colorspace { DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED, DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT, DRM_MODE_COLORIMETRY_BT601_YCC, + DRM_MODE_COLORIMETRY_MAX }; /** @@ -1818,8 +1820,10 @@ int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state *old_state, struct drm_connector_state *new_state); int drm_mode_create_aspect_ratio_property(struct drm_device *dev); -int drm_mode_create_hdmi_colorspace_property(struct drm_connector *connector); -int drm_mode_create_dp_colorspace_property(struct drm_connector *connector); +int drm_mode_create_hdmi_colorspace_property(struct drm_connector *connector, + u32 supported_colorspaces); +int drm_mode_create_dp_colorspace_property(struct drm_connector *connector, + u32 supported_colorspaces); int drm_mode_create_content_type_property(struct drm_device *dev); int drm_mode_create_suggested_offset_properties(struct drm_device *dev); -- 2.39.0