This patch set splits out static hdr metadata backlight range parsing from gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c into gpu/drm/drm-edid.c as a new function. This new function is then used during edid parsing when HDR static metadata block parsing.
Calculated values are stored in a new struct drm_luminance_range introduced into display_info. Amdgpu_dm.c and intel_dp_aux_backlight.c are using this new data.
v2: Calculate the range during edid parsing and store into display_info
Cc: Roman Li roman.li@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Rodrigo Siqueira Rodrigo.Siqueira@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: Lyude Paul lyude@redhat.com Cc: Mika Kahola mika.kahola@intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Manasi Navare manasi.d.navare@intel.com
Jouni Högander (4): drm/display: Add drm_luminance_range_info drm: New function to get luminance range based on static hdr metadata drm/amdgpu_dm: Rely on split out luminance calculation function drm/i915: Use luminance range calculated during edid parsing
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 35 ++----------- drivers/gpu/drm/drm_edid.c | 50 ++++++++++++++++++- .../drm/i915/display/intel_dp_aux_backlight.c | 17 +++++-- include/drm/drm_connector.h | 21 ++++++++ 4 files changed, 88 insertions(+), 35 deletions(-)
Add new data structure to store luminance range calculated using data from EDID's static hdr metadata block. Add this new struct as a part of drm_display_info struct.
Cc: Roman Li roman.li@amd.com Cc: Rodrigo Siqueira Rodrigo.Siqueira@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: Lyude Paul lyude@redhat.com Cc: Mika Kahola mika.kahola@intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com --- include/drm/drm_connector.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 3ac4bf87f257..7d8eeac6cc68 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -322,6 +322,22 @@ struct drm_monitor_range_info { u8 max_vfreq; };
+/** + * struct drm_luminance_range_info - Panel's luminance range for + * &drm_display_info. Calculated using data in EDID + * + * This struct is used to store a luminance range supported by panel + * as calculated using data from EDID's static hdr metadata. + * + * @min_luminance: This is the min supported luminance value + * + * @max_luminance: This is the max supported luminance value + */ +struct drm_luminance_range_info { + u32 min_luminance; + u32 max_luminance; +}; + /** * enum drm_privacy_screen_status - privacy screen status * @@ -623,6 +639,11 @@ struct drm_display_info { */ struct drm_monitor_range_info monitor_range;
+ /** + * @luminance_range: Luminance range supported by panel + */ + struct drm_luminance_range_info luminance_range; + /** * @mso_stream_count: eDP Multi-SST Operation (MSO) stream count from * the DisplayID VESA vendor block. 0 for conventional Single-Stream
On Mon, 20 Jun 2022, Jouni Högander jouni.hogander@intel.com wrote:
Add new data structure to store luminance range calculated using data from EDID's static hdr metadata block. Add this new struct as a part of drm_display_info struct.
Cc: Roman Li roman.li@amd.com Cc: Rodrigo Siqueira Rodrigo.Siqueira@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: Lyude Paul lyude@redhat.com Cc: Mika Kahola mika.kahola@intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com
LGTM but I think I'd just fold this into the next patch that actually uses it.
BR, Jani.
include/drm/drm_connector.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 3ac4bf87f257..7d8eeac6cc68 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -322,6 +322,22 @@ struct drm_monitor_range_info { u8 max_vfreq; };
+/**
- struct drm_luminance_range_info - Panel's luminance range for
- &drm_display_info. Calculated using data in EDID
- This struct is used to store a luminance range supported by panel
- as calculated using data from EDID's static hdr metadata.
- @min_luminance: This is the min supported luminance value
- @max_luminance: This is the max supported luminance value
- */
+struct drm_luminance_range_info {
- u32 min_luminance;
- u32 max_luminance;
+};
/**
- enum drm_privacy_screen_status - privacy screen status
@@ -623,6 +639,11 @@ struct drm_display_info { */ struct drm_monitor_range_info monitor_range;
- /**
* @luminance_range: Luminance range supported by panel*/- struct drm_luminance_range_info luminance_range;
- /**
- @mso_stream_count: eDP Multi-SST Operation (MSO) stream count from
- the DisplayID VESA vendor block. 0 for conventional Single-Stream
Split luminance min/max calculation using static hdr metadata from drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:update_connector_ext_caps into drm/drm_edid.c and use it during edid parsing. Calculated range is stored into connector->display_info->luminance_range.
v2: Calculate range during edid parsing
Cc: Roman Li roman.li@amd.com Cc: Rodrigo Siqueira Rodrigo.Siqueira@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: Lyude Paul lyude@redhat.com Cc: Mika Kahola mika.kahola@intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com --- drivers/gpu/drm/drm_edid.c | 50 +++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2bdaf1e34a9d..3b367100290f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5014,6 +5014,49 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) mode->clock = clock; }
+static void drm_calculate_luminance_range(struct drm_connector *connector) +{ + struct hdr_static_metadata *hdr_metadata = &connector->hdr_sink_metadata.hdmi_type1; + struct drm_luminance_range_info *luminance_range = + &connector->display_info.luminance_range; + static const u8 pre_computed_values[] = { + 50, 51, 52, 53, 55, 56, 57, 58, 59, 61, 62, 63, 65, 66, 68, 69, + 71, 72, 74, 75, 77, 79, 81, 82, 84, 86, 88, 90, 92, 94, 96, 98}; + u32 max_avg, min_cll, max, min, q, r; + + if (!(hdr_metadata->metadata_type & BIT(HDMI_STATIC_METADATA_TYPE1))) + return; + + max_avg = hdr_metadata->max_fall; + min_cll = hdr_metadata->min_cll; + + /* From the specification (CTA-861-G), for calculating the maximum + * luminance we need to use: + * Luminance = 50*2**(CV/32) + * Where CV is a one-byte value. + * For calculating this expression we may need float point precision; + * to avoid this complexity level, we take advantage that CV is divided + * by a constant. From the Euclids division algorithm, we know that CV + * can be written as: CV = 32*q + r. Next, we replace CV in the + * Luminance expression and get 50*(2**q)*(2**(r/32)), hence we just + * need to pre-compute the value of r/32. For pre-computing the values + * We just used the following Ruby line: + * (0...32).each {|cv| puts (50*2**(cv/32.0)).round} + * The results of the above expressions can be verified at + * pre_computed_values. + */ + q = max_avg >> 5; + r = max_avg % 32; + max = (1 << q) * pre_computed_values[r]; + + /* min luminance: maxLum * (CV/255)^2 / 100 */ + q = DIV_ROUND_CLOSEST(min_cll, 255); + min = max * DIV_ROUND_CLOSEST((q * q), 100); + + luminance_range->min_luminance = min; + luminance_range->max_luminance = max; +} + static uint8_t eotf_supported(const u8 *edid_ext) { return edid_ext[2] & @@ -5045,8 +5088,12 @@ drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; if (len >= 5) connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; - if (len >= 6) + if (len >= 6) { connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; + + /* Calculate only when all values are available */ + drm_calculate_luminance_range(connector); + } }
static void @@ -5951,6 +5998,7 @@ drm_reset_display_info(struct drm_connector *connector)
info->non_desktop = 0; memset(&info->monitor_range, 0, sizeof(info->monitor_range)); + memset(&info->luminance_range, 0, sizeof(info->luminance_range));
info->mso_stream_count = 0; info->mso_pixel_overlap = 0;
On Mon, 20 Jun 2022, Jouni Högander jouni.hogander@intel.com wrote:
Split luminance min/max calculation using static hdr metadata from drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:update_connector_ext_caps into drm/drm_edid.c and use it during edid parsing. Calculated range is stored into connector->display_info->luminance_range.
v2: Calculate range during edid parsing
Cc: Roman Li roman.li@amd.com Cc: Rodrigo Siqueira Rodrigo.Siqueira@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: Lyude Paul lyude@redhat.com Cc: Mika Kahola mika.kahola@intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com
drivers/gpu/drm/drm_edid.c | 50 +++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2bdaf1e34a9d..3b367100290f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5014,6 +5014,49 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) mode->clock = clock; }
+static void drm_calculate_luminance_range(struct drm_connector *connector) +{
- struct hdr_static_metadata *hdr_metadata = &connector->hdr_sink_metadata.hdmi_type1;
- struct drm_luminance_range_info *luminance_range =
&connector->display_info.luminance_range;- static const u8 pre_computed_values[] = {
50, 51, 52, 53, 55, 56, 57, 58, 59, 61, 62, 63, 65, 66, 68, 69,71, 72, 74, 75, 77, 79, 81, 82, 84, 86, 88, 90, 92, 94, 96, 98};
Nitpick, newline before };
- u32 max_avg, min_cll, max, min, q, r;
- if (!(hdr_metadata->metadata_type & BIT(HDMI_STATIC_METADATA_TYPE1)))
return;- max_avg = hdr_metadata->max_fall;
- min_cll = hdr_metadata->min_cll;
- /* From the specification (CTA-861-G), for calculating the maximum
Nitpick, newline after /*.
* luminance we need to use:* Luminance = 50*2**(CV/32)* Where CV is a one-byte value.* For calculating this expression we may need float point precision;* to avoid this complexity level, we take advantage that CV is divided* by a constant. From the Euclids division algorithm, we know that CV* can be written as: CV = 32*q + r. Next, we replace CV in the* Luminance expression and get 50*(2**q)*(2**(r/32)), hence we just* need to pre-compute the value of r/32. For pre-computing the values* We just used the following Ruby line:* (0...32).each {|cv| puts (50*2**(cv/32.0)).round}* The results of the above expressions can be verified at* pre_computed_values.*/- q = max_avg >> 5;
- r = max_avg % 32;
- max = (1 << q) * pre_computed_values[r];
- /* min luminance: maxLum * (CV/255)^2 / 100 */
- q = DIV_ROUND_CLOSEST(min_cll, 255);
- min = max * DIV_ROUND_CLOSEST((q * q), 100);
- luminance_range->min_luminance = min;
- luminance_range->max_luminance = max;
I'll just trust this; not going to start verifying.
Acked-by: Jani Nikula jani.nikula@intel.com
+}
static uint8_t eotf_supported(const u8 *edid_ext) { return edid_ext[2] & @@ -5045,8 +5088,12 @@ drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; if (len >= 5) connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5];
- if (len >= 6)
- if (len >= 6) { connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6];
/* Calculate only when all values are available */drm_calculate_luminance_range(connector);- }
}
static void @@ -5951,6 +5998,7 @@ drm_reset_display_info(struct drm_connector *connector)
info->non_desktop = 0; memset(&info->monitor_range, 0, sizeof(info->monitor_range));
memset(&info->luminance_range, 0, sizeof(info->luminance_range));
info->mso_stream_count = 0; info->mso_pixel_overlap = 0;
Luminance range calculation was split out into drm_edid.c and is now part of edid parsing. Rely on values calculated during edid parsing and use these for caps->aux_max_input_signal and caps->aux_min_input_signal.
v2: Use values calculated during edid parsing
Cc: Roman Li roman.li@amd.com Cc: Rodrigo Siqueira Rodrigo.Siqueira@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: Lyude Paul lyude@redhat.com Cc: Mika Kahola mika.kahola@intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 35 +++---------------- 1 file changed, 4 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 428bb041f92c..896a6b5eb2ed 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2813,15 +2813,12 @@ static struct drm_mode_config_helper_funcs amdgpu_dm_mode_config_helperfuncs = {
static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector) { - u32 max_avg, min_cll, max, min, q, r; struct amdgpu_dm_backlight_caps *caps; struct amdgpu_display_manager *dm; struct drm_connector *conn_base; struct amdgpu_device *adev; struct dc_link *link = NULL; - static const u8 pre_computed_values[] = { - 50, 51, 52, 53, 55, 56, 57, 58, 59, 61, 62, 63, 65, 66, 68, 69, - 71, 72, 74, 75, 77, 79, 81, 82, 84, 86, 88, 90, 92, 94, 96, 98}; + struct drm_luminance_range_info *luminance_range; int i;
if (!aconnector || !aconnector->dc_link) @@ -2843,8 +2840,6 @@ static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector) caps = &dm->backlight_caps[i]; caps->ext_caps = &aconnector->dc_link->dpcd_sink_ext_caps; caps->aux_support = false; - max_avg = conn_base->hdr_sink_metadata.hdmi_type1.max_fall; - min_cll = conn_base->hdr_sink_metadata.hdmi_type1.min_cll;
if (caps->ext_caps->bits.oled == 1 /*|| caps->ext_caps->bits.sdr_aux_backlight_control == 1 || @@ -2856,31 +2851,9 @@ static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector) else if (amdgpu_backlight == 1) caps->aux_support = true;
- /* From the specification (CTA-861-G), for calculating the maximum - * luminance we need to use: - * Luminance = 50*2**(CV/32) - * Where CV is a one-byte value. - * For calculating this expression we may need float point precision; - * to avoid this complexity level, we take advantage that CV is divided - * by a constant. From the Euclids division algorithm, we know that CV - * can be written as: CV = 32*q + r. Next, we replace CV in the - * Luminance expression and get 50*(2**q)*(2**(r/32)), hence we just - * need to pre-compute the value of r/32. For pre-computing the values - * We just used the following Ruby line: - * (0...32).each {|cv| puts (50*2**(cv/32.0)).round} - * The results of the above expressions can be verified at - * pre_computed_values. - */ - q = max_avg >> 5; - r = max_avg % 32; - max = (1 << q) * pre_computed_values[r]; - - // min luminance: maxLum * (CV/255)^2 / 100 - q = DIV_ROUND_CLOSEST(min_cll, 255); - min = max * DIV_ROUND_CLOSEST((q * q), 100); - - caps->aux_max_input_signal = max; - caps->aux_min_input_signal = min; + luminance_range = &conn_base->display_info.luminance_range; + caps->aux_min_input_signal = luminance_range->min_luminance; + caps->aux_max_input_signal = luminance_range->max_luminance; }
void amdgpu_dm_update_connector_after_detect(
Instead of using fixed 0 - 512 range use luminance range calculated as a part of edid parsing. As a backup fall back to static 0 - 512.
v2: Use values calculated during edid parsing
Cc: Lyude Paul lyude@redhat.com Cc: Mika Kahola mika.kahola@intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com --- .../drm/i915/display/intel_dp_aux_backlight.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c index c92d5bb2326a..b2666bd67701 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -276,8 +276,11 @@ intel_dp_aux_hdr_disable_backlight(const struct drm_connector_state *conn_state, static int intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pipe) { - struct drm_i915_private *i915 = to_i915(connector->base.dev); + struct drm_connector *conn_base = &connector->base; + struct drm_i915_private *i915 = to_i915(conn_base->dev); struct intel_panel *panel = &connector->panel; + struct drm_luminance_range_info *luminance_range = + &conn_base->display_info.luminance_range; int ret;
if (panel->backlight.edp.intel.sdr_uses_aux) { @@ -293,8 +296,16 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi } }
- panel->backlight.max = 512; - panel->backlight.min = 0; + if (!luminance_range->max_luminance) { + panel->backlight.max = 512; + panel->backlight.min = 0; + } else { + panel->backlight.max = luminance_range->max_luminance; + panel->backlight.min = luminance_range->min_luminance; + } + + drm_dbg(&i915->drm, "Using range %d..%d\n", panel->backlight.min, panel->backlight.max); + panel->backlight.level = intel_dp_aux_hdr_get_backlight(connector, pipe); panel->backlight.enabled = panel->backlight.level != 0;
On Mon, 20 Jun 2022, Jouni Högander jouni.hogander@intel.com wrote:
Instead of using fixed 0 - 512 range use luminance range calculated as a part of edid parsing. As a backup fall back to static 0 - 512.
v2: Use values calculated during edid parsing
Cc: Lyude Paul lyude@redhat.com Cc: Mika Kahola mika.kahola@intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Signed-off-by: Jouni Högander jouni.hogander@intel.com
.../drm/i915/display/intel_dp_aux_backlight.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c index c92d5bb2326a..b2666bd67701 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -276,8 +276,11 @@ intel_dp_aux_hdr_disable_backlight(const struct drm_connector_state *conn_state, static int intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pipe) {
- struct drm_i915_private *i915 = to_i915(connector->base.dev);
- struct drm_connector *conn_base = &connector->base;
Nitpick, we never call this "conn_base". Usually we try to avoid having drm_connector as local variable altogether, but if you must, please name it _connector or drm_connector.
struct drm_i915_private *i915 = to_i915(conn_base->dev); struct intel_panel *panel = &connector->panel;
struct drm_luminance_range_info *luminance_range =
&conn_base->display_info.luminance_range;int ret;
if (panel->backlight.edp.intel.sdr_uses_aux) {
@@ -293,8 +296,16 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi } }
- panel->backlight.max = 512;
- panel->backlight.min = 0;
- if (!luminance_range->max_luminance) {
Nitpick, I kind of dislike these "if (!something) ... else ..." constructs when you could avoid the negative by switching it around.
panel->backlight.max = 512;panel->backlight.min = 0;- } else {
panel->backlight.max = luminance_range->max_luminance;panel->backlight.min = luminance_range->min_luminance;- }
- drm_dbg(&i915->drm, "Using range %d..%d\n", panel->backlight.min, panel->backlight.max);
Please use drm_dbg_kms(). Please say *what* range this is. Otherwise, you have to figure it out from the context and function names.
- panel->backlight.level = intel_dp_aux_hdr_get_backlight(connector, pipe); panel->backlight.enabled = panel->backlight.level != 0;
dri-devel@lists.freedesktop.org
-
Jani Nikula -
Jouni Högander