From 4425bcd4e0b98a91c3500afe7204fec22b9032f6 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Fri, 22 Dec 2023 02:03:46 +0100 Subject: [PATCH] backends/drm: handle missing brightness values in the EDID better Most importantly, fall back to an assumption of 1000 nits peak brightness when the max brightness is missing, instead of the sdr brightness, which causes more immediately visible issues. Ideally the user should configure this value in the display settings, but it's too late to still do that for Plasma 6.0. BUG: 478860 --- src/backends/drm/drm_output.cpp | 13 +++++-------- src/backends/drm/drm_pipeline.cpp | 26 ++++++++++---------------- src/core/output.cpp | 8 ++++---- src/core/output.h | 14 +++++++------- src/utils/edid.cpp | 29 ++++++++++++++++++++++++----- src/utils/edid.h | 23 +++++++++++++---------- src/wayland/outputdevice_v2.cpp | 6 +++--- 7 files changed, 66 insertions(+), 53 deletions(-) diff --git a/src/backends/drm/drm_output.cpp b/src/backends/drm/drm_output.cpp index 75ce4f9d1f..7097ac7931 100644 --- a/src/backends/drm/drm_output.cpp +++ b/src/backends/drm/drm_output.cpp @@ -59,11 +59,10 @@ DrmOutput::DrmOutput(const std::shared_ptr &conn) capabilities |= Capability::RgbRange; initialState.rgbRange = DrmConnector::broadcastRgbToRgbRange(conn->broadcastRGB.enumValue()); } - if (m_connector->hdrMetadata.isValid() && m_connector->edid() && m_connector->edid()->hdrMetadata() && m_connector->edid()->hdrMetadata()->supportsPQ) { + if (m_connector->hdrMetadata.isValid() && m_connector->edid()->supportsPQ()) { capabilities |= Capability::HighDynamicRange; } - if (m_connector->colorspace.isValid() && m_connector->colorspace.hasEnum(DrmConnector::Colorspace::BT2020_RGB) - && m_connector->edid() && m_connector->edid()->hdrMetadata() && m_connector->edid()->hdrMetadata()->supportsBT2020) { + if (m_connector->colorspace.isValid() && m_connector->colorspace.hasEnum(DrmConnector::Colorspace::BT2020_RGB) && m_connector->edid()->supportsBT2020()) { capabilities |= Capability::WideColorGamut; } if (conn->isInternal()) { @@ -72,8 +71,6 @@ DrmOutput::DrmOutput(const std::shared_ptr &conn) } const Edid *edid = conn->edid(); - - const auto hdrData = edid->hdrMetadata(); setInformation(Information{ .name = conn->connectorName(), .manufacturer = edid->manufacturerString(), @@ -88,9 +85,9 @@ DrmOutput::DrmOutput(const std::shared_ptr &conn) .internal = conn->isInternal(), .nonDesktop = conn->isNonDesktop(), .mstPath = conn->mstPath(), - .maxPeakBrightness = hdrData && hdrData->hasValidBrightnessValues ? hdrData->desiredContentMaxLuminance : 0, - .maxAverageBrightness = hdrData && hdrData->hasValidBrightnessValues ? hdrData->desiredMaxFrameAverageLuminance : 0, - .minBrightness = hdrData && hdrData->hasValidBrightnessValues ? hdrData->desiredContentMinLuminance : 0, + .maxPeakBrightness = edid->desiredMaxLuminance(), + .maxAverageBrightness = edid->desiredMaxFrameAverageLuminance(), + .minBrightness = edid->desiredMinLuminance(), }); initialState.modes = getModes(); diff --git a/src/backends/drm/drm_pipeline.cpp b/src/backends/drm/drm_pipeline.cpp index c0f8cd36df..1a8d149ea8 100644 --- a/src/backends/drm/drm_pipeline.cpp +++ b/src/backends/drm/drm_pipeline.cpp @@ -769,13 +769,11 @@ ColorDescription DrmPipeline::createColorDescription() const { if (m_pending.transferFunction == NamedTransferFunction::PerceptualQuantizer && m_connector->edid()) { const auto colorimetry = m_pending.BT2020 ? NamedColorimetry::BT2020 : NamedColorimetry::BT709; - if (const auto hdr = m_connector->edid()->hdrMetadata(); hdr && hdr->hasValidBrightnessValues) { - return ColorDescription(colorimetry, m_pending.transferFunction, m_pending.sdrBrightness, hdr->desiredContentMinLuminance, hdr->desiredMaxFrameAverageLuminance, hdr->desiredContentMaxLuminance, m_pending.sdrGamutWideness); - } else if (m_pending.peakBrightnessOverride && m_pending.averageBrightnessOverride) { - return ColorDescription(colorimetry, m_pending.transferFunction, m_pending.sdrBrightness, m_pending.minBrightnessOverride.value_or(0), *m_pending.averageBrightnessOverride, *m_pending.peakBrightnessOverride, m_pending.sdrGamutWideness); - } else { - return ColorDescription(colorimetry, m_pending.transferFunction, m_pending.sdrBrightness, 0, m_pending.sdrBrightness, m_pending.sdrBrightness, m_pending.sdrGamutWideness); - } + return ColorDescription(colorimetry, m_pending.transferFunction, m_pending.sdrBrightness, + m_pending.minBrightnessOverride.value_or(m_connector->edid()->desiredMinLuminance()), + m_pending.averageBrightnessOverride.value_or(m_connector->edid()->desiredMaxFrameAverageLuminance().value_or(m_pending.sdrBrightness)), + m_pending.peakBrightnessOverride.value_or(m_connector->edid()->desiredMaxLuminance().value_or(1000)), + m_pending.sdrGamutWideness); } else if (m_pending.iccProfile) { return ColorDescription(m_pending.iccProfile->colorimetry(), NamedTransferFunction::sRGB, 200, 0, 200, 200, 0); } else { @@ -789,11 +787,7 @@ std::shared_ptr DrmPipeline::createHdrMetadata(NamedTransferFunction tr // for sRGB / gamma 2.2, don't send any metadata, to ensure the non-HDR experience stays the same return nullptr; } - if (!m_connector->edid() || !m_connector->edid()->hdrMetadata()) { - return nullptr; - } - const auto metadata = *m_connector->edid()->hdrMetadata(); - if (!metadata.supportsPQ) { + if (!m_connector->edid()->supportsPQ()) { return nullptr; } const auto colorimetry = m_connector->edid()->colorimetry(); @@ -820,12 +814,12 @@ std::shared_ptr DrmPipeline::createHdrMetadata(NamedTransferFunction tr }, .white_point = {to16Bit(colorimetry.white.x()), to16Bit(colorimetry.white.y())}, // in nits - .max_display_mastering_luminance = uint16_t(std::round(metadata.desiredMaxFrameAverageLuminance)), + .max_display_mastering_luminance = uint16_t(std::round(m_connector->edid()->desiredMaxFrameAverageLuminance().value_or(0))), // in 0.0001 nits - .min_display_mastering_luminance = uint16_t(std::round(metadata.desiredContentMinLuminance * 10000)), + .min_display_mastering_luminance = uint16_t(std::round(m_connector->edid()->desiredMaxLuminance().value_or(0) * 10000)), // in nits - .max_cll = uint16_t(std::round(metadata.desiredMaxFrameAverageLuminance)), - .max_fall = uint16_t(std::round(metadata.desiredMaxFrameAverageLuminance)), + .max_cll = uint16_t(std::round(m_connector->edid()->desiredMaxFrameAverageLuminance().value_or(0))), + .max_fall = uint16_t(std::round(m_connector->edid()->desiredMaxFrameAverageLuminance().value_or(0))), }, }; return DrmBlob::create(gpu(), &data, sizeof(data)); diff --git a/src/core/output.cpp b/src/core/output.cpp index f0ea308987..f4afcff82d 100644 --- a/src/core/output.cpp +++ b/src/core/output.cpp @@ -624,14 +624,14 @@ const ColorDescription &Output::colorDescription() const return m_state.colorDescription; } -double Output::maxPeakBrightness() const +std::optional Output::maxPeakBrightness() const { - return m_state.maxPeakBrightnessOverride.value_or(m_information.maxPeakBrightness); + return m_state.maxPeakBrightnessOverride ? m_state.maxPeakBrightnessOverride : m_information.maxPeakBrightness; } -double Output::maxAverageBrightness() const +std::optional Output::maxAverageBrightness() const { - return m_state.maxAverageBrightnessOverride.value_or(m_information.maxAverageBrightness); + return m_state.maxAverageBrightnessOverride ? *m_state.maxAverageBrightnessOverride : m_information.maxAverageBrightness; } double Output::minBrightness() const diff --git a/src/core/output.h b/src/core/output.h index 849f0dd500..6bdb88f62a 100644 --- a/src/core/output.h +++ b/src/core/output.h @@ -341,8 +341,8 @@ public: virtual bool updateCursorLayer(); - double maxPeakBrightness() const; - double maxAverageBrightness() const; + std::optional maxPeakBrightness() const; + std::optional maxAverageBrightness() const; double minBrightness() const; std::optional maxPeakBrightnessOverride() const; std::optional maxAverageBrightnessOverride() const; @@ -431,8 +431,8 @@ protected: bool placeholder = false; bool nonDesktop = false; QByteArray mstPath; - double maxPeakBrightness = 0; - double maxAverageBrightness = 0; + std::optional maxPeakBrightness; + std::optional maxAverageBrightness; double minBrightness = 0; }; @@ -456,9 +456,9 @@ protected: QString iccProfilePath; std::shared_ptr iccProfile; ColorDescription colorDescription = ColorDescription::sRGB; - std::optional maxPeakBrightnessOverride; - std::optional maxAverageBrightnessOverride; - std::optional minBrightnessOverride; + std::optional maxPeakBrightnessOverride; + std::optional maxAverageBrightnessOverride; + std::optional minBrightnessOverride; double sdrGamutWideness = 0; }; diff --git a/src/utils/edid.cpp b/src/utils/edid.cpp index 30d73d6681..a04c208be2 100644 --- a/src/utils/edid.cpp +++ b/src/utils/edid.cpp @@ -166,10 +166,9 @@ Edid::Edid(const void *data, uint32_t size) } if (hdr_static_metadata) { m_hdrMetadata = HDRMetadata{ - .hasValidBrightnessValues = hdr_static_metadata->desired_content_min_luminance > 0 && hdr_static_metadata->desired_content_max_luminance > 0 && hdr_static_metadata->desired_content_max_frame_avg_luminance, .desiredContentMinLuminance = hdr_static_metadata->desired_content_min_luminance, - .desiredContentMaxLuminance = hdr_static_metadata->desired_content_max_luminance, - .desiredMaxFrameAverageLuminance = hdr_static_metadata->desired_content_max_frame_avg_luminance, + .desiredContentMaxLuminance = hdr_static_metadata->desired_content_max_luminance > 0 ? std::make_optional(hdr_static_metadata->desired_content_max_luminance) : std::nullopt, + .desiredMaxFrameAverageLuminance = hdr_static_metadata->desired_content_max_frame_avg_luminance > 0 ? std::make_optional(hdr_static_metadata->desired_content_max_frame_avg_luminance) : std::nullopt, .supportsPQ = hdr_static_metadata->eotfs->pq, .supportsBT2020 = colorimetry && colorimetry->bt2020_rgb, }; @@ -252,9 +251,29 @@ Colorimetry Edid::colorimetry() const return m_colorimetry; } -std::optional Edid::hdrMetadata() const +double Edid::desiredMinLuminance() const { - return m_hdrMetadata; + return m_hdrMetadata ? m_hdrMetadata->desiredContentMinLuminance : 0; +} + +std::optional Edid::desiredMaxFrameAverageLuminance() const +{ + return m_hdrMetadata ? m_hdrMetadata->desiredMaxFrameAverageLuminance : std::nullopt; +} + +std::optional Edid::desiredMaxLuminance() const +{ + return m_hdrMetadata ? m_hdrMetadata->desiredContentMaxLuminance : std::nullopt; +} + +bool Edid::supportsPQ() const +{ + return m_hdrMetadata && m_hdrMetadata->supportsPQ; +} + +bool Edid::supportsBT2020() const +{ + return m_hdrMetadata && m_hdrMetadata->supportsBT2020; } QByteArray Edid::identifier() const diff --git a/src/utils/edid.h b/src/utils/edid.h index d2424b5460..bf22a20654 100644 --- a/src/utils/edid.h +++ b/src/utils/edid.h @@ -81,16 +81,11 @@ public: Colorimetry colorimetry() const; - struct HDRMetadata - { - bool hasValidBrightnessValues; - float desiredContentMinLuminance; - float desiredContentMaxLuminance; - float desiredMaxFrameAverageLuminance; - bool supportsPQ; - bool supportsBT2020; - }; - std::optional hdrMetadata() const; + double desiredMinLuminance() const; + std::optional desiredMaxFrameAverageLuminance() const; + std::optional desiredMaxLuminance() const; + bool supportsPQ() const; + bool supportsBT2020() const; /** * @returns a string that is intended to identify the monitor uniquely. @@ -106,6 +101,14 @@ private: QByteArray m_serialNumber; QString m_hash; Colorimetry m_colorimetry; + struct HDRMetadata + { + double desiredContentMinLuminance; + std::optional desiredContentMaxLuminance; + std::optional desiredMaxFrameAverageLuminance; + bool supportsPQ; + bool supportsBT2020; + }; std::optional m_hdrMetadata; QByteArray m_identifier; diff --git a/src/wayland/outputdevice_v2.cpp b/src/wayland/outputdevice_v2.cpp index 7f5737cc3e..2c2ed35fcf 100644 --- a/src/wayland/outputdevice_v2.cpp +++ b/src/wayland/outputdevice_v2.cpp @@ -133,8 +133,8 @@ public: bool m_wideColorGamut = false; auto_rotate_policy m_autoRotation = auto_rotate_policy::auto_rotate_policy_in_tablet_mode; QString m_iccProfilePath; - double m_maxPeakBrightness = 0; - double m_maxAverageBrightness = 0; + std::optional m_maxPeakBrightness; + std::optional m_maxAverageBrightness; double m_minBrightness = 0; double m_sdrGamutWideness = 0; std::optional m_maxPeakBrightnessOverride; @@ -445,7 +445,7 @@ void OutputDeviceV2InterfacePrivate::sendIccProfilePath(Resource *resource) void OutputDeviceV2InterfacePrivate::sendBrightnessMetadata(Resource *resource) { if (resource->version() >= KDE_OUTPUT_DEVICE_V2_BRIGHTNESS_METADATA_SINCE_VERSION) { - send_brightness_metadata(resource->handle, std::round(m_maxPeakBrightness), std::round(m_maxAverageBrightness), std::round(m_minBrightness * 10'000)); + send_brightness_metadata(resource->handle, std::round(m_maxPeakBrightness.value_or(0)), std::round(m_maxAverageBrightness.value_or(0)), std::round(m_minBrightness * 10'000)); } }