drm/amd/display: Stop rounding backlight levels to whole percents
Subsystems (3)
cover drm/amd/display: Stop rounding backlight levels to whole percents
Both backlight paths in amdgpu_dm_backlight.c map the 16-bit user level through the ATIF custom brightness curve, whose points pair an 8-bit signal with a whole-percent luminance. Both round the level to one of those signals and the luminance to a whole percent first. That hurts most at the dim end, where one percent is a large share of the light. With the curve of an ASUS ROG Flow X13 GV301QH, minimum 12 and 49 points:
- On PWM, the lowest level asks for 2% of full brightness, and the first 2% of the scale steps from there to 3%, 4% and 5%. Each step is 50%, 33% and 25% brighter than the one before. The bottom 10% of the scale has 18 different brightness values
- On AUX, with amdgpu.backlight=1 to force the unsupported AUX path, the 1..512 nits fallback range stays at 1 nit for the bottom 6.1% of the scale, and the next steps go to 3, 5 and 7 nits. The bottom 10% of the scale has 4 different values, and the top stops at 496 nits
This series interpolates on the unrounded level and rounds once at the end. On PWM, the power module also interpolates between its LUT entries now, from millipercent to PWM and back. The bottom 10% of the scale then has 6018 different values on PWM instead of 18, and 33311 on AUX instead of 4. Level 0 still gives the panel's minimum and the top level gives its maximum. On AUX only levels 0 and 1 now sit at the minimum, instead of the bottom 6.1% of the scale
- Patch 1 renames the min and max of the user range to user_min and user_max, to avoid confusion with the curve's signal_max and the output_max that patch 4 adds. No functional change
- Patches 2 and 3 interpolate between the power module's LUT entries, from millipercent to PWM and back
- Patch 4 stops rounding the PWM path
- Patches 5 and 6 test the PWM path and the LUT with the GV301QH curve
- Patch 7 maps AUX through the same curve code as PWM, which reaches max at the top, and removes convert_custom_brightness()
- Patch 8 tests the AUX path
Patch 7 changes what an AUX panel gets. Until now an AUX level asked for the curve's luminance percentage times the level, so the level went in twice. Now it asks for min plus that percentage of the range above min, the same luminance the PWM path asks the power module for. On the fallback range, half the scale goes from 159 nits to 315 nits. I found no reason for the old product, but I may be missing the intent. I also can't test it meaningfully, as I don't have a panel with an AUX backlight
The series applies on top of "drm/amd/display: Fix the doubled AUX backlight minimum on the custom curve" [1]. That series fixes the minimum going in twice on AUX, and asks whether the ATIF curve is meant for AUX at all. Patch 7 keeps applying the curve on AUX, as the code always has. Since it changes what AUX panels get, it has no Fixes: tags
One question for you: on PWM, above the last point the curve still runs to 100% at max_input_signal, as backlight_signal_to_luminance() did, while the level maps to the 0..255 signal scale over the whole user range. With max_input_signal below 255 the top of the PWM scale is flat at full brightness. AUX keeps ending the curve at 255, as it did before. Should PWM end at 255 too?
This is also a request for testing. My laptop drives its panel over PWM, and the panel ignores AMD's AUX brightness, so I tested the AUX path only with KUnit and with the amdgpu_dm_brightness trace on a forced AUX path. If you have a panel with an AUX backlight and an ATIF custom curve, a Tested-by on patches 7 and 8 would help. The middle of the scale should get brighter, the bottom should no longer sit at the minimum and the top should reach full brightness. Patches 1 to 6 don't depend on them, so they can go in without the AUX half
[1] https://lore.kernel.org/r/[email protected]
---
Hugo Sales (8):
drm/amd/display: Rename the user brightness range to user_min and user_max drm/amd/display: Interpolate between backlight LUT entries drm/amd/display: Interpolate when reading PWM back as a percentage drm/amd/display: Stop rounding PWM brightness to whole percents drm/amd/display: Add KUnit tests for the custom brightness curve drm/amd/display: Add KUnit tests for backlight LUT interpolation drm/amd/display: Map AUX brightness through the custom curve like PWM drm/amd/display: Add KUnit tests for the AUX custom brightness curve
.../amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 218 ++--- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.h | 6 +- .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 923 +++++++++++++++------ .../gpu/drm/amd/display/modules/power/power_abm.c | 72 +- .../drm/amd/display/modules/power/power_helpers.h | 4 + 5 files changed, 784 insertions(+), 439 deletions(-)
---
base-commit: ba496bb837882eb1af58f41572fc289e46c77b8c
change-id: 20261102-backlight-curve-rfc-1bb4b6dee24c
prerequisite-message-id: <[email protected]>
prerequisite-patch-id: 0eefd25bf4ad8f6d779c7a0b0a63f84969b67fca
prerequisite-patch-id: 016515673f79f623de1e50dd513e5440587329b6
prerequisite-patch-id: ba453baf376b5410d66feacb1956eea933042ae8
Best regards,
Signature · 1 line
Hugo Sales <[email protected]>
RFC PATCH 1/8 drm/amd/display: Rename the user brightness range to user_min and user_max
get_brightness_range() gives the range of the user brightness level, and the code around it calls its two ends min and max. The custom curve code later in this series adds an input signal range and an output range next to it, so call this one user_min and user_max
No functional change
---
Rename only. It also joins two lines in amdgpu_dm_backlight_fill_props() that now fit in 100 columns, and wraps two test comments that no longer do
amdgpu_dm_backlight.o disassembles the same with and without this patch. amdgpu_dm_backlight_test.o differs only in the line numbers its KUnit asserts record, since the two wrapped comments add two lines
KUnit: amdgpu_dm_backlight passes 74 of 74 with and without this patch, and the tests from patches 5, 6 and 8 give the same results with and without it
--- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 72 ++++---- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.h | 4 +- .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 194 +++++++++++---------- 3 files changed, 135 insertions(+), 135 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c +35 -37
@@ -98,34 +98,34 @@ EXPORT_IF_KUNIT(amdgpu_dm_update_backlight_caps);STATIC_IFN_KUNIT int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps, -unsigned int *min,unsigned int *max) +unsigned int *user_min,unsigned int *user_max) {if (!caps)return 0 ;if (caps->aux_support) {/* Firmware limits are in nits, DC API wants millinits. */ - *max =1000 * caps->aux_max_input_signal; - *min =1000 * caps->aux_min_input_signal; + *user_max =1000 * caps->aux_max_input_signal; + *user_min =1000 * caps->aux_min_input_signal; }else {/* Firmware limits are 8-bit, PWM control is 16-bit. */ - *max =0x101 * caps->max_input_signal; - *min =0x101 * caps->min_input_signal; + *user_max =0x101 * caps->max_input_signal; + *user_min =0x101 * caps->min_input_signal; }return 1 ; }EXPORT_IF_KUNIT (get_brightness_range); -/* Rescale userspace [0..max] to the firmware curve's [0..255]. */ -static inline u32 scale_input_to_fw (int max,u64 input) +/* Rescale userspace [0..user_max] to the firmware curve's [0..255]. */ +static inline u32 scale_input_to_fw (int user_max,u64 input) { -return DIV_ROUND_CLOSEST_ULL (input *AMDGPU_MAX_BL_LEVEL , max); +return DIV_ROUND_CLOSEST_ULL (input *AMDGPU_MAX_BL_LEVEL , user_max); } -/* Rescale from [0..AMDGPU_MAX_BL_LEVEL] to [0..max] */ -static inline u32 scale_fw_to_input (int max,u64 input) +/* Rescale from [0..AMDGPU_MAX_BL_LEVEL] to [0..user_max] */ +static inline u32 scale_fw_to_input (int user_max,u64 input) { -return DIV_ROUND_CLOSEST_ULL (input * max,AMDGPU_MAX_BL_LEVEL ); +return DIV_ROUND_CLOSEST_ULL (input * user_max,AMDGPU_MAX_BL_LEVEL ); } static u32 backlight_signal_to_luminance(const struct amdgpu_dm_backlight_caps *caps,
@@ -173,18 +173,18 @@ static u32 backlight_signal_to_luminance(const struct amdgpu_dm_backlight_caps *static u32 convert_brightness_to_millipercent (const struct amdgpu_dm_backlight_caps *caps,u32 user_brightness) { -unsigned int min, max; +unsigned int user_min, user_max;u32 signal, luminance; -if (!get_brightness_range (caps, &min, &max)) +if (!get_brightness_range (caps, &user_min, &user_max))return DIV_ROUND_CLOSEST_ULL ((u64 )user_brightness *100000 ,MAX_BACKLIGHT_LEVEL ); - user_brightness =clamp (user_brightness, min, max); + user_brightness =clamp (user_brightness, user_min, user_max);if (amdgpu_dc_debug_mask &DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE ) -return DIV_ROUND_CLOSEST_ULL ((u64 )user_brightness *100000 , max); +return DIV_ROUND_CLOSEST_ULL ((u64 )user_brightness *100000 , user_max); - signal =scale_input_to_fw (max, user_brightness); + signal =scale_input_to_fw (user_max, user_brightness); luminance =backlight_signal_to_luminance (caps, signal);return min_t (u32, luminance *1000 ,100000 );
@@ -192,9 +192,9 @@ static u32 convert_brightness_to_millipercent(const struct amdgpu_dm_backlight_cSTATIC_IFN_KUNIT void convert_custom_brightness(const struct amdgpu_dm_backlight_caps *caps, -unsigned int max,uint32_t *user_brightness) +unsigned int user_max,uint32_t *user_brightness) { -u32 brightness =scale_input_to_fw (max, *user_brightness); +u32 brightness =scale_input_to_fw (user_max, *user_brightness);u8 lower_signal, upper_signal, upper_lum, lower_lum, lum;int left, right;
@@ -250,7 +250,7 @@ void convert_custom_brightness(const struct amdgpu_dm_backlight_caps *caps, (brightness - lower_signal), upper_signal - lower_signal); scale: - *user_brightness =scale_fw_to_input (max,DIV_ROUND_CLOSEST (lum * brightness,101 )); + *user_brightness =scale_fw_to_input (user_max,DIV_ROUND_CLOSEST (lum * brightness,101 )); }EXPORT_IF_KUNIT (convert_custom_brightness);
@@ -259,15 +259,15 @@ STATIC_IFN_KUNITu32 convert_brightness_from_user (const struct amdgpu_dm_backlight_caps *caps,uint32_t brightness) { -unsigned int min, max; +unsigned int user_min, user_max; -if (!get_brightness_range (caps, &min, &max)) +if (!get_brightness_range (caps, &user_min, &user_max))return brightness; -convert_custom_brightness (caps, max, &brightness); +convert_custom_brightness (caps, user_max, &brightness); -/* Rescale 0..max to min..max */ -return min +DIV_ROUND_CLOSEST_ULL ((u64)(max - min) * brightness, max); +/* Rescale 0..user_max to user_min..user_max */ +return user_min +DIV_ROUND_CLOSEST_ULL ((u64)(user_max - user_min) * brightness, user_max); }EXPORT_IF_KUNIT (convert_brightness_from_user);
@@ -276,16 +276,16 @@ STATIC_IFN_KUNITu32 convert_brightness_to_user (const struct amdgpu_dm_backlight_caps *caps,uint32_t brightness) { -unsigned int min, max; +unsigned int user_min, user_max; -if (!get_brightness_range (caps, &min, &max)) +if (!get_brightness_range (caps, &user_min, &user_max))return brightness; -if (brightness < min) +if (brightness < user_min)return 0 ; -/* Rescale min..max to 0..max */ -return DIV_ROUND_CLOSEST_ULL ((u64 )max * (brightness - min), - max - min); +/* Rescale user_min..user_max to 0..user_max */ +return DIV_ROUND_CLOSEST_ULL ((u64 )user_max * (brightness - user_min), + user_max - user_min); }EXPORT_IF_KUNIT (convert_brightness_to_user);
@@ -484,16 +484,14 @@ void amdgpu_dm_backlight_fill_props(const struct amdgpu_dm_backlight_caps *caps,bool custom_curve_enabled, struct backlight_properties *props) { - unsigned int min, max; + unsigned int user_min, user_max; -if (get_brightness_range (caps, &min, &max)) { +if (get_brightness_range (caps, &user_min, &user_max)) {if (is_system_supplied) - props->brightness =DIV_ROUND_CLOSEST (max * caps->ac_level, -100 ); + props->brightness =DIV_ROUND_CLOSEST (user_max * caps->ac_level,100 );else - props->brightness =DIV_ROUND_CLOSEST (max * caps->dc_level, -100 ); - props->max_brightness = max; + props->brightness =DIV_ROUND_CLOSEST (user_max * caps->dc_level,100 ); + props->max_brightness = user_max; }else { props->brightness =MAX_BACKLIGHT_LEVEL ; props->max_brightness =MAX_BACKLIGHT_LEVEL ;
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.h +2 -2
@@ -66,9 +66,9 @@ ssize_t panel_power_savings_store(struct device *device,struct device_attribute *attr, const char *buf, size_t count);int get_brightness_range (const struct amdgpu_dm_backlight_caps *caps, -unsigned int *min,unsigned int *max); +unsigned int *user_min,unsigned int *user_max);void convert_custom_brightness (const struct amdgpu_dm_backlight_caps *caps, -unsigned int max,uint32_t *user_brightness); +unsigned int user_max,uint32_t *user_brightness);u32 convert_brightness_from_user (const struct amdgpu_dm_backlight_caps *caps,uint32_t brightness); u32 convert_brightness_to_user(const struct amdgpu_dm_backlight_caps *caps,
drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c +98 -96
530 lines not on this page · raw message
Signature · 1 line
2.56.0
RFC PATCH 2/8 drm/amd/display: Interpolate between backlight LUT entries
In reply to Hugo Salesbacklight_millipercent_to_pwm_legacy() takes the floor index into backlight_lut, which has one entry per percent of luminance. Any request between two whole percents gets the PWM of the lower one, so a PWM panel has at most 101 brightness levels however fine the request is
Interpolate linearly between the entries on each side of the request. Whole percents still give the LUT entry. The result never decreases as the request grows, since validate_ext_backlight_caps() only accepts a curve whose signals and luminances strictly increase, so the LUT never decreases either. The linear curve bypass and the 100% case are unchanged
---amdgpu_dm only hands the power module whole percents until patch 4, so on its own this patch changes no PWM for it. Coming first keeps patch 4 from dropping a level to the whole percent below
With patch 4 too, on the GV301QH curve with minimum 12, the 62452 levels from user_min to user_max reach the panel as 57821 different PWM values. Without the series they give 99, and patch 4 without this patch would give 100. The bottom tenth of the scale goes from 18 PWM values to 5434
BACKLIGHT_MILLIPERCENT_MAX is defined at the top of power_abm.c as 100 * 1000. mod_power_create() sets num_backlight_levels to at least 101 and allocates backlight_lut with that many entries, and nothing changes it afterwards, so index + 1 stays within the LUT. The function still returns early with fewer than 2 levels, as backlight_pwm_to_millipercent_legacy() already does
KUnit: dm_test_pwm_lut_between_percents, dm_test_pwm_lut_monotonic and dm_test_pwm_lut_min_input_signal from patch 6 fail without this patch and pass with it
--- .../gpu/drm/amd/display/modules/power/power_abm.c | 29 ++++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-)
drivers/gpu/drm/amd/display/modules/power/power_abm.c +24 -5
@@ -524,6 +524,9 @@ static unsigned int backlight_millipercent_to_pwm_legacy( {unsigned int pwm = (unsigned int )-1 ;unsigned int index =0 ; +unsigned int lower, upper; +unsigned long long scaled; +unsigned int remainder;if (core_power ==NULL )return 0 ;
@@ -541,17 +544,33 @@ static unsigned int backlight_millipercent_to_pwm_legacy(return pwm; } -if (millipercent >= (100 *1000 )) +if (millipercent >=BACKLIGHT_MILLIPERCENT_MAX )return core_power->bl_prop[inst].backlight_lut[core_power->bl_prop[inst].num_backlight_levels -1 ]; -/* This will give the floor index. */ - index = ((core_power->bl_prop[inst].num_backlight_levels -1 ) * - millipercent) /100000 ;/* Null check otherwise eDP doesn't lightup when connected to DP1 */ if (core_power->bl_prop[inst].backlight_lut ==NULL )return pwm; - pwm = core_power->bl_prop[inst].backlight_lut[index]; +if (core_power->bl_prop[inst].num_backlight_levels <2 ) +return pwm; + +/* +* Interpolate between the two LUT entries around millipercent. +* backlight_lut has num_backlight_levels entries, and millipercent is +* below BACKLIGHT_MILLIPERCENT_MAX here, so index + 1 stays within it +*/ + scaled = (unsigned long long )(core_power->bl_prop[inst].num_backlight_levels -1 ) * + millipercent; + index = (unsigned int )div_u64_rem (scaled,BACKLIGHT_MILLIPERCENT_MAX , &remainder); + lower = core_power->bl_prop[inst].backlight_lut[index]; + upper = core_power->bl_prop[inst].backlight_lut[index +1 ]; + +if (upper <= lower) +return lower; + + pwm = lower + (unsigned int )div_u64 ((unsigned long long )(upper - lower) * remainder + +BACKLIGHT_MILLIPERCENT_MAX /2 , +BACKLIGHT_MILLIPERCENT_MAX );return pwm; }
Signature · 1 line
2.56.0
Re: [PATCH RFC RFT 2/8] drm/amd/display: Interpolate between backlight LUT entries
Quoted 91 lines · Hugo Sales
backlight_millipercent_to_pwm_legacy()takes the floor index intobacklight_lut, which has one entry per percent of luminance. Any request between two whole percents gets the PWM of the lower one, so a PWM panel has at most 101 brightness levels however fine the request isInterpolate linearly between the entries on each side of the request. Whole percents still give the LUT entry. The result never decreases as the request grows, since
validate_ext_backlight_caps()only accepts a curve whose signals and luminances strictly increase, so the LUT never decreases either. The linear curve bypass and the 100% case are unchangedSigned-off-by Hugo Sales <[email protected]>---
amdgpu_dmonly hands the power module whole percents until patch 4, so on its own this patch changes no PWM for it. Coming first keeps patch 4 from dropping a level to the whole percent belowWith patch 4 too, on the GV301QH curve with minimum 12, the 62452 levels from
user_mintouser_maxreach the panel as 57821 different PWM values. Without the series they give 99, and patch 4 without this patch would give 100. The bottom tenth of the scale goes from 18 PWM values to 5434
BACKLIGHT_MILLIPERCENT_MAXis defined at the top ofpower_abm.cas 100 * 1000.mod_power_create()setsnum_backlight_levelsto at least 101 and allocatesbacklight_lutwith that many entries, and nothing changes it afterwards, so index + 1 stays within the LUT. The function still returns early with fewer than 2 levels, asbacklight_pwm_to_millipercent_legacy()already doesKUnit:
dm_test_pwm_lut_between_percents,dm_test_pwm_lut_monotonicanddm_test_pwm_lut_min_input_signalfrom patch 6 fail without this patch and pass with it--- .../gpu/drm/amd/display/modules/power/power_abm.c | 29 ++++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-)drivers/gpu/drm/amd/display/modules/power/power_abm.c +24
-5@@ -524,6 +524,9 @@ static unsigned int backlight_millipercent_to_pwm_legacy( {unsigned int pwm = (unsigned int )-1 ;unsigned int index =0 ; +unsigned int lower, upper; +unsigned long long scaled; +unsigned int remainder;if (core_power ==NULL )return 0 ;@@ -541,17 +544,33 @@ static unsigned int backlight_millipercent_to_pwm_legacy(return pwm; } -if (millipercent >= (100 *1000 )) +if (millipercent >=BACKLIGHT_MILLIPERCENT_MAX )return core_power->bl_prop[inst].backlight_lut[core_power->bl_prop[inst].num_backlight_levels -1 ]; -/* This will give the floor index. */ - index = ((core_power->bl_prop[inst].num_backlight_levels -1 ) * - millipercent) /100000 ;/* Null check otherwise eDP doesn't lightup when connected to DP1 */ if (core_power->bl_prop[inst].backlight_lut ==NULL )return pwm; - pwm = core_power->bl_prop[inst].backlight_lut[index]; +if (core_power->bl_prop[inst].num_backlight_levels <2 ) +return pwm; + +/* +* Interpolate between the two LUT entries around millipercent. +* backlight_lut has num_backlight_levels entries, and millipercent is +* below BACKLIGHT_MILLIPERCENT_MAX here, so index + 1 stays within it +*/ + scaled = (unsigned long long )(core_power->bl_prop[inst].num_backlight_levels -1 ) * + millipercent; + index = (unsigned int )div_u64_rem (scaled,BACKLIGHT_MILLIPERCENT_MAX , &remainder); + lower = core_power->bl_prop[inst].backlight_lut[index]; + upper = core_power->bl_prop[inst].backlight_lut[index +1 ]; + +if (upper <= lower) +return lower; + + pwm = lower + (unsigned int )div_u64 ((unsigned long long )(upper - lower) * remainder + +BACKLIGHT_MILLIPERCENT_MAX /2 , +BACKLIGHT_MILLIPERCENT_MAX );return pwm; }
RFC PATCH 3/8 drm/amd/display: Interpolate when reading PWM back as a percentage
In reply to Hugo Salesbacklight_pwm_to_millipercent_legacy() finds the closest backlight_lut entry and returns its whole percent. That was the exact inverse while backlight_millipercent_to_pwm_legacy() only returned LUT entries. Now that it interpolates, a request of 1.5% programs a PWM halfway between two entries, which reads back as 1% or 2%, and setting the value read back moves the backlight by up to half a LUT step
Interpolate between the two entries around the PWM instead. A PWM equal to a LUT entry still reads back as that entry's whole percent
---
Converting a percentage to PWM, back, and to PWM again now moves the PWM by about a two-thousandth of the LUT step around it, plus rounding. On the GV301QH LUT, whose steps are at most 1799, that keeps every PWM within 1 of where it started. A curve with much wider steps can move further, but by far less than the half step it could before
KUnit: dm_test_pwm_lut_readback from patch 6 fails without this patch and passes with it
--- .../gpu/drm/amd/display/modules/power/power_abm.c | 33 ++++++++-------------- 1 file changed, 11 insertions(+), 22 deletions(-)
drivers/gpu/drm/amd/display/modules/power/power_abm.c +11 -22
@@ -409,34 +409,23 @@ static unsigned int backlight_pwm_to_millipercent_legacy(break ; } -/* In this case, exact match is not found. Check if mid/min/max -* value is actually closer. -*/ +/* No exact match, so on a sorted LUT lut[max] < pwm < lut[min] */ if (max < min) { -unsigned int min_delta; -unsigned int mid_delta; -unsigned int max_delta; - - min_delta = (core_power->bl_prop[inst].backlight_lut[min] > pwm) ? - core_power->bl_prop[inst].backlight_lut[min] - pwm : - pwm - core_power->bl_prop[inst].backlight_lut[min]; - - mid_delta = (core_power->bl_prop[inst].backlight_lut[mid] > pwm) ? - core_power->bl_prop[inst].backlight_lut[mid] - pwm : - pwm - core_power->bl_prop[inst].backlight_lut[mid]; +unsigned int lower = core_power->bl_prop[inst].backlight_lut[max]; +unsigned int upper = core_power->bl_prop[inst].backlight_lut[min]; +unsigned long long span, den; - max_delta = (core_power->bl_prop[inst].backlight_lut[max] > pwm) ? - core_power->bl_prop[inst].backlight_lut[max] - pwm : - pwm - core_power->bl_prop[inst].backlight_lut[max]; +if (pwm <= lower || pwm >= upper) +return BACKLIGHT_MILLIPERCENT_MAX * mid / max_index; -if ((min_delta < mid_delta) && (min_delta < max_delta)) - mid = min; + span = upper - lower; + den = span * max_index; -if ((max_delta < mid_delta) && (max_delta < min_delta)) - mid = max; +/* LUT entries fit in 16 bits, so this stays far below 2^64 */ +return (unsigned int )div64_u64 ((max * span + pwm - lower) * +BACKLIGHT_MILLIPERCENT_MAX + den /2 , den); } -/* No interpolation, just take closest index */ millipercent =BACKLIGHT_MILLIPERCENT_MAX * mid / max_index;return millipercent;
Signature · 1 line
2.56.0
Re: [PATCH RFC RFT 3/8] drm/amd/display: Interpolate when reading PWM back as a percentage
Quoted 75 lines · Hugo Sales
backlight_pwm_to_millipercent_legacy()finds the closestbacklight_lutentry and returns its whole percent. That was the exact inverse whilebacklight_millipercent_to_pwm_legacy()only returned LUT entries. Now that it interpolates, a request of 1.5% programs a PWM halfway between two entries, which reads back as 1% or 2%, and setting the value read back moves the backlight by up to half a LUT stepInterpolate between the two entries around the PWM instead. A PWM equal to a LUT entry still reads back as that entry's whole percent
Signed-off-by Hugo Sales <[email protected]>---
Converting a percentage to PWM, back, and to PWM again now moves the PWM by about a two-thousandth of the LUT step around it, plus rounding. On the GV301QH LUT, whose steps are at most 1799, that keeps every PWM within 1 of where it started. A curve with much wider steps can move further, but by far less than the half step it could beforeKUnit:
dm_test_pwm_lut_readbackfrom patch 6 fails without this patch and passes with it--- .../gpu/drm/amd/display/modules/power/power_abm.c | 33 ++++++++-------------- 1 file changed, 11 insertions(+), 22 deletions(-)drivers/gpu/drm/amd/display/modules/power/power_abm.c +11
-22@@ -409,34 +409,23 @@ static unsigned int backlight_pwm_to_millipercent_legacy(break ; } -/* In this case, exact match is not found. Check if mid/min/max -* value is actually closer. -*/ +/* No exact match, so on a sorted LUT lut[max] < pwm < lut[min] */ if (max < min) { -unsigned int min_delta; -unsigned int mid_delta; -unsigned int max_delta; - - min_delta = (core_power->bl_prop[inst].backlight_lut[min] > pwm) ? - core_power->bl_prop[inst].backlight_lut[min] - pwm : - pwm - core_power->bl_prop[inst].backlight_lut[min]; - - mid_delta = (core_power->bl_prop[inst].backlight_lut[mid] > pwm) ? - core_power->bl_prop[inst].backlight_lut[mid] - pwm : - pwm - core_power->bl_prop[inst].backlight_lut[mid]; +unsigned int lower = core_power->bl_prop[inst].backlight_lut[max]; +unsigned int upper = core_power->bl_prop[inst].backlight_lut[min]; +unsigned long long span, den; - max_delta = (core_power->bl_prop[inst].backlight_lut[max] > pwm) ? - core_power->bl_prop[inst].backlight_lut[max] - pwm : - pwm - core_power->bl_prop[inst].backlight_lut[max]; +if (pwm <= lower || pwm >= upper) +return BACKLIGHT_MILLIPERCENT_MAX * mid / max_index; -if ((min_delta < mid_delta) && (min_delta < max_delta)) - mid = min; + span = upper - lower; + den = span * max_index; -if ((max_delta < mid_delta) && (max_delta < min_delta)) - mid = max; +/* LUT entries fit in 16 bits, so this stays far below 2^64 */ +return (unsigned int )div64_u64 ((max * span + pwm - lower) * +BACKLIGHT_MILLIPERCENT_MAX + den /2 , den); } -/* No interpolation, just take closest index */ millipercent =BACKLIGHT_MILLIPERCENT_MAX * mid / max_index;return millipercent;
RFC PATCH 4/8 drm/amd/display: Stop rounding PWM brightness to whole percents
In reply to Hugo SalesSince commit d422a66847a9 ("drm/amd/display: map PWM brightness through custom backlight curve"), convert_brightness_to_millipercent() rounds the 16-bit user level to an 8-bit signal and the luminance to a whole percent, so the power module gets at most 101 levels
Interpolate on the unrounded user level and round once, to millipercent. The curve and its segments stay the same, and a result over 100% is still capped
---
On the GV301QH curve, minimum 12 and 49 points, the 62452 levels from user_min to user_max gave 99 different millipercent values and now give 62112
The curve's first point is 2% at signal 14, and below it the curve is a line from 0, so the minimum, signal 12, should get 2% * 12 / 14, which is 1714 millipercent. It now gets exactly that instead of 2000, and every other level also gets the curve's value instead of the nearest whole percent
Since patches 2 and 3 the power module's LUT keeps those steps. On the same curve the levels reach the panel as 57821 different PWM values instead of 99
backlight_custom_curve_scale() maps 0..user_max to 0..output_max, with the curve reaching 100% at signal_max. The two weights of the interpolation can't go negative. A luminance is a u8, so before the cap the result is at most 2.55 times output_max. The divisor can pass 32 bits, hence mul_u64_add_u64_div_u64()
With no data points the mapping stays linear. Above the last point the curve still runs to 100% at max_input_signal, and points above max_input_signal are still interpolated as before. Past a last point above max_input_signal the curve runs on to 100% at signal 255. Before, the divisor wrapped there and the level stayed near that point's luminance. The top level still gives 100% when a point at max_input_signal claims less
KUnit: the PWM tests from patch 5 fail without this patch and pass with it
--- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 79 ++++++++++++---------- 1 file changed, 42 insertions(+), 37 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c +42 -37
@@ -39,6 +39,7 @@#include "modules/inc/mod_power.h" #include <linux/backlight.h> +#include <linux/math64.h> #include <linux/minmax.h> #include <linux/power_supply.h> #include <drm/drm_edid.h>
@@ -128,53 +129,59 @@ static inline u32 scale_fw_to_input(int user_max, u64 input)return DIV_ROUND_CLOSEST_ULL (input * user_max,AMDGPU_MAX_BL_LEVEL ); } -static u32 backlight_signal_to_luminance (const struct amdgpu_dm_backlight_caps *caps, -u32 signal) +/* +* Map user level 0..user_max to 0..output_max through the custom curve, +* which ends at 100% at signal_max, or at AMDGPU_MAX_BL_LEVEL when the +* last point lies past signal_max. A point at signal s sits at user +* level s * user_max / AMDGPU_MAX_BL_LEVEL, so compare +* user * AMDGPU_MAX_BL_LEVEL against s * user_max rather than rounding the +* user level to 8 bits +*/ +static u32 backlight_custom_curve_scale (const struct amdgpu_dm_backlight_caps *caps, +u32 user,u32 user_max,u32 signal_max, +u32 output_max) { -u32 lower_signal, upper_signal; -u32 lower_lum, upper_lum; +u64 x, x0 =0 , x1, num, den; +u32 l0 =0 , l1 =100 ;int i; + user =min (user, user_max);if (!caps->data_points) -return DIV_ROUND_CLOSEST (signal *100 ,AMDGPU_MAX_BL_LEVEL ); - -if (signal < caps->luminance_data[0 ].input_signal) { - lower_signal =0 ; - lower_lum =0 ; - upper_signal = caps->luminance_data[0 ].input_signal; - upper_lum = caps->luminance_data[0 ].luminance; - }else if (signal >= - caps->luminance_data[caps->data_points -1 ].input_signal) { - lower_signal = caps->luminance_data[caps->data_points -1 ].input_signal; - lower_lum = caps->luminance_data[caps->data_points -1 ].luminance; - upper_signal = caps->max_input_signal; - upper_lum =100 ; +return DIV_ROUND_CLOSEST_ULL ((u64 )user * output_max, user_max); + + x = (u64 )user *AMDGPU_MAX_BL_LEVEL ; +for (i =0 ; i < caps->data_points; i++) { +u64 xi = (u64 )caps->luminance_data[i].input_signal * user_max; + +if (x < xi) +break ; + x0 = xi; + l0 = caps->luminance_data[i].luminance; + } + +if (i < caps->data_points) { + x1 = (u64 )caps->luminance_data[i].input_signal * user_max; + l1 = caps->luminance_data[i].luminance; }else { -for (i =1 ; - i < caps->data_points && - signal > caps->luminance_data[i].input_signal; - i++) - ; - - lower_signal = caps->luminance_data[i -1 ].input_signal; - upper_signal = caps->luminance_data[i].input_signal; - lower_lum = caps->luminance_data[i -1 ].luminance; - upper_lum = caps->luminance_data[i].luminance; + x1 = (u64 )signal_max * user_max; +if (x0 > x1) + x1 = (u64 )AMDGPU_MAX_BL_LEVEL * user_max; +if (x >= x1) +return output_max; } -if (upper_signal == lower_signal) -return upper_lum; +/* x0 <= x < x1, so neither weight wraps */ + num = (u64 )l0 * (x1 - x) + (u64 )l1 * (x - x0); + den =100 * (x1 - x0); -return lower_lum +DIV_ROUND_CLOSEST ((upper_lum - lower_lum) * - (signal - lower_signal), - upper_signal - lower_signal); +return min_t (u64,mul_u64_add_u64_div_u64 (output_max, num, den /2 , den), + output_max); }static u32 convert_brightness_to_millipercent (const struct amdgpu_dm_backlight_caps *caps,u32 user_brightness) {unsigned int user_min, user_max; -u32 signal, luminance;if (!get_brightness_range (caps, &user_min, &user_max)) return DIV_ROUND_CLOSEST_ULL((u64)user_brightness * 100000,
@@ -184,10 +191,8 @@ static u32 convert_brightness_to_millipercent(const struct amdgpu_dm_backlight_cif (amdgpu_dc_debug_mask &DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE )return DIV_ROUND_CLOSEST_ULL ((u64 )user_brightness *100000 , user_max); - signal =scale_input_to_fw (user_max, user_brightness); - luminance =backlight_signal_to_luminance (caps, signal); - -return min_t (u32, luminance *1000 ,100000 ); +return backlight_custom_curve_scale (caps, user_brightness, user_max, + caps->max_input_signal,100000 ); }STATIC_IFN_KUNIT
Signature · 1 line
2.56.0
RFC PATCH 5/8 drm/amd/display: Add KUnit tests for the custom brightness curve
In reply to Hugo SalesAdd KUnit tests for the PWM path, using the curve from the ATIF reply of an ASUS ROG Flow X13 GV301QH: minimum 12 and 49 points. They check values worked out by hand, and walk every request to check that the millipercent never goes down and changes on most steps. Two more curves cover points above max_input_signal and a luminance over 100%
Export convert_brightness_to_millipercent() when the KUnit tests are enabled, so the tests can call it
---
KUnit on UML with PROVE_LOCKING: amdgpu_dm_backlight passes 74 of 74 without this patch and 78 of 78 with it. Patch 4 says which of the new tests fail without it
The other amdgpu_dm suites behave the same with and without the series. Both runs stop in amdgpu_dm_crtc, at the BUG in drm_vblank_restore() that dm_test_crtc_enable_vblank_ips_restore hits. With that test skipped, the crtc, helpers, quirks and services suites pass
W=1 C=1 on power_abm, amdgpu_dm, amdgpu_dm_backlight, amdgpu_dm_connector, amdgpu_dm_services and the backlight and connector test objects adds no warnings at any patch, and checkpatch --strict is clean
--- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 7 +- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.h | 2 + .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 212 +++++++++++++++++++++ 3 files changed, 219 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c +5 -2
@@ -178,8 +178,9 @@ static u32 backlight_custom_curve_scale(const struct amdgpu_dm_backlight_caps *c output_max); } -static u32 convert_brightness_to_millipercent (const struct amdgpu_dm_backlight_caps *caps, -u32 user_brightness) +STATIC_IFN_KUNIT +u32 convert_brightness_to_millipercent(const struct amdgpu_dm_backlight_caps *caps, +u32 user_brightness) {unsigned int user_min, user_max;
@@ -195,6 +196,8 @@ static u32 convert_brightness_to_millipercent(const struct amdgpu_dm_backlight_c caps->max_input_signal, 100000); } +EXPORT_IF_KUNIT (convert_brightness_to_millipercent); +STATIC_IFN_KUNIT void convert_custom_brightness(const struct amdgpu_dm_backlight_caps *caps,unsigned int user_max,uint32_t *user_brightness)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.h +2 -0
@@ -69,6 +69,8 @@ int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps, unsigned int *user_min, unsigned int *user_max);void convert_custom_brightness (const struct amdgpu_dm_backlight_caps *caps,unsigned int user_max,uint32_t *user_brightness); +u32 convert_brightness_to_millipercent (const struct amdgpu_dm_backlight_caps *caps, +u32 user_brightness);u32 convert_brightness_from_user (const struct amdgpu_dm_backlight_caps *caps,uint32_t brightness); u32 convert_brightness_to_user(const struct amdgpu_dm_backlight_caps *caps,
drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c +212 -0
224 lines not on this page · raw message
Signature · 1 line
2.56.0
RFC PATCH 6/8 drm/amd/display: Add KUnit tests for backlight LUT interpolation
In reply to Hugo SalesBuild the power module's LUT with initialize_backlight_caps() from the ATIF reply of an ASUS ROG Flow X13 GV301QH, with min_input_signal 0, and test backlight_millipercent_to_pwm() on it. A whole percent gives its LUT entry, and a request in between lands between the two entries, at values worked out by hand. The PWM never goes down as the request goes up, and below the first curve point every millipercent gets its own PWM, where the floor index gave only 0 and 0x707
Test backlight_pwm_to_millipercent() by hand at two points too, and check that every PWM turned into a percentage and back lands within 1 of where it started. A LUT built with the real min_input_signal of 12 gets the same hand checks in both directions
Export these functions when the KUnit tests are enabled, so the tests can call them
---
The tests use gv301qh_curve and set_gv301qh_caps() from patch 5
KUnit on UML with PROVE_LOCKING: amdgpu_dm_backlight passes 78 of 78 without this patch and 83 of 83 with it. dm_test_pwm_lut_whole_percents passes at every patch, since a whole percent always gave the LUT entry. dm_test_pwm_lut_between_percents, dm_test_pwm_lut_monotonic and dm_test_pwm_lut_min_input_signal pass from patch 2, and dm_test_pwm_lut_readback from patch 3
The other amdgpu_dm suites behave the same with and without the series, as in patch 5
checkpatch --strict flags two prototypes whose line ends in '('. They follow the DC style of the prototypes around them in power_abm.c and power_helpers.h
--- .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 146 +++++++++++++++++++++ .../gpu/drm/amd/display/modules/power/power_abm.c | 10 +- .../drm/amd/display/modules/power/power_helpers.h | 4 + 3 files changed, 159 insertions(+), 1 deletion(-)
drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c +146 -0
@@ -24,6 +24,7 @@#include "amd_shared.h" #include "link_service.h" #include "dc/inc/hw/panel_cntl.h" +#include "power_helpers.h" struct dm_backlight_connector_fixture {struct amdgpu_device *adev;
@@ -2163,6 +2164,145 @@ static void dm_test_setup_backlight_device_attaches_abm_property(struct kunit *tamdgpu_dm_set_abm_level_param (saved_abm_level); } +/* Tests for the legacy backlight LUT conversion, in both directions */ + +/* +* Build the power module's backlight LUT from the GV301QH curve the same +* way the driver does, through the ATIF caps cached in the display manager +*/ +static struct core_power *gv301qh_power_module (struct kunit *test,int min_input_signal) +{ +struct amdgpu_device *adev =dm_kunit_alloc_adev (test); +struct dc *dc =dm_kunit_alloc_dc_with_ctx (test); +struct core_power *core_power; + + core_power =kunit_kzalloc (test,sizeof (*core_power),GFP_KERNEL ); +KUNIT_ASSERT_NOT_NULL (test, core_power); + +set_gv301qh_caps (&adev->dm.backlight_caps[0 ], false); + adev->dm.backlight_caps[0 ].min_input_signal = min_input_signal; + adev->dm.backlight_caps[0 ].caps_valid = true; + dc->ctx->driver_context = adev; + core_power->dc = dc; + core_power->bl_prop[0 ].num_backlight_levels =101 ; + core_power->bl_prop[0 ].backlight_lut =kunit_kcalloc (test,101 ,sizeof (unsigned int ), +GFP_KERNEL ); +KUNIT_ASSERT_NOT_NULL (test, core_power->bl_prop[0 ].backlight_lut); + +initialize_backlight_caps (core_power,0 ); +KUNIT_ASSERT_TRUE (test, core_power->bl_prop[0 ].backlight_caps_valid); + +return core_power; +} + +/** +* dm_test_pwm_lut_whole_percents - Test Whole percents give the LUT entry +* @test: The KUnit test context +* +* With min_input_signal 0 the LUT starts at 0, reaches the first curve +* point (signal 14, 2%) at index 2 and has 0x707 halfway at index 1. Index +* 50 is the point (98, 50%) +*/ +static void dm_test_pwm_lut_whole_percents (struct kunit *test) +{ +struct core_power *core_power =gv301qh_power_module (test,0 ); + +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,0 ,0 ),0U ); +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,1000 ,0 ),0x707U ); +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,2000 ,0 ),0xe0eU ); +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,50000 ,0 ),98U *0x101 ); +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,100000 ,0 ),0xffffU ); +} + +/** +* dm_test_pwm_lut_between_percents - Test Requests between percents interpolate +* @test: The KUnit test context +* +* The code rounds the step between two entries before adding it. 0.5% is +* halfway from 0 to 0x707, so 1799 / 2 = 899.5, which rounds to 900. 1.5% +* is 1799 + 900 = 2699. 2.5% is halfway from 0xe0e to 0xf0f, so 257 / 2 = +* 128.5 rounds to 129 and 3598 + 129 = 3727. 99.999% is 1028 * 999 / 1000 +* = 1026.97 past 64507, which rounds to 1027 and gives 65534 +*/ +static void dm_test_pwm_lut_between_percents (struct kunit *test) +{ +struct core_power *core_power =gv301qh_power_module (test,0 ); + +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,500 ,0 ),900U ); +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,1500 ,0 ),2699U ); +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,2500 ,0 ),3727U ); +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,99999 ,0 ),65534U ); +} + +/** +* dm_test_pwm_lut_readback - Test PWM reads back as the percentage that set it +* @test: The KUnit test context +* +* 900 is halfway from 0 to 0x707, so it reads back as 0.5%. Every PWM from +* 0 to 0xffff turned into a percentage and back must land within 1 of +* where it started +*/ +static void dm_test_pwm_lut_readback (struct kunit *test) +{ +struct core_power *core_power =gv301qh_power_module (test,0 ); +unsigned int pwm, mp; + +KUNIT_EXPECT_EQ (test,backlight_pwm_to_millipercent (core_power,0x707 ,0 ),1000U ); +KUNIT_EXPECT_EQ (test,backlight_pwm_to_millipercent (core_power,900 ,0 ),500U ); + +for (pwm =0 ; pwm <=0xffff ; pwm++) { + mp =backlight_pwm_to_millipercent (core_power, pwm,0 ); +KUNIT_ASSERT_LE_MSG (test, +abs ((int )backlight_millipercent_to_pwm (core_power, mp,0 ) - + (int )pwm),1 , +"pwm %u" , pwm); + } +} + +/** +* dm_test_pwm_lut_monotonic - Test PWM never goes down as the request goes up +* @test: The KUnit test context +* +* Every millipercent below 2% must also get its own PWM, where the floor +* index gave only 0 and 0x707 +*/ +static void dm_test_pwm_lut_monotonic (struct kunit *test) +{ +struct core_power *core_power =gv301qh_power_module (test,0 ); +unsigned int mp, pwm, prev =0 , steps =0 ; + +for (mp =0 ; mp <=100000 ; mp++) { + pwm =backlight_millipercent_to_pwm (core_power, mp,0 ); +KUNIT_ASSERT_GE_MSG (test, pwm, prev,"millipercent %u" , mp); +if (mp <2000 && (mp ==0 || pwm != prev)) + steps++; + prev = pwm; + } + +KUNIT_EXPECT_EQ (test, steps,2000U ); +} + +/** +* dm_test_pwm_lut_min_input_signal - Test the LUT with the real minimum of 12 +* @test: The KUnit test context +* +* With min_input_signal 12 the LUT starts at 0xc0c, has 3341 at index 1 and +* reaches the first curve point (signal 14, 2%) at index 2, 0xe0e. 1.714%, +* the curve's value at the minimum, is 0.714 of the way from 3341 to 0xe0e, +* so 3341 + 183 = 3524. A PWM at or below 0xc0c reads back as 0% +*/ +static void dm_test_pwm_lut_min_input_signal (struct kunit *test) +{ +struct core_power *core_power =gv301qh_power_module (test,12 ); + +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,0 ,0 ),0xc0cU ); +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,1000 ,0 ),3341U ); +KUNIT_EXPECT_EQ (test,backlight_millipercent_to_pwm (core_power,1714 ,0 ),3524U ); +KUNIT_EXPECT_EQ (test,backlight_pwm_to_millipercent (core_power,3341 ,0 ),1000U ); +KUNIT_EXPECT_EQ (test,backlight_pwm_to_millipercent (core_power,0xc0c ,0 ),0U ); +KUNIT_EXPECT_EQ (test,backlight_pwm_to_millipercent (core_power,1000 ,0 ),0U ); +} +static struct kunit_case dm_backlight_test_cases[] = {/* dm_find_stream_with_link */ KUNIT_CASE (dm_test_find_stream_with_link_returns_match),
@@ -2255,6 +2395,12 @@ static struct kunit_case dm_backlight_test_cases[] = {KUNIT_CASE (dm_test_setup_backlight_device_max_edps),KUNIT_CASE (dm_test_setup_backlight_device_oled_success),KUNIT_CASE (dm_test_setup_backlight_device_attaches_abm_property), +/* legacy backlight LUT conversion */ +KUNIT_CASE (dm_test_pwm_lut_whole_percents), +KUNIT_CASE (dm_test_pwm_lut_between_percents), +KUNIT_CASE (dm_test_pwm_lut_readback), +KUNIT_CASE (dm_test_pwm_lut_monotonic), + KUNIT_CASE(dm_test_pwm_lut_min_input_signal), {} };
drivers/gpu/drm/amd/display/modules/power/power_abm.c +9 -1
@@ -3,6 +3,7 @@// Copyright 2026 Advanced Micro Devices, Inc. #include "dm_services.h" +#include "dm_helpers.h" #include "dc.h" #include "mod_power.h" #include "core_types.h"
@@ -436,7 +437,8 @@ static unsigned int backlight_pwm_to_millipercent_legacy(* 0 PWM = 0% and max_backlight_pwm = 100%. * Otherwise, falls back to the legacy LUT based mapping. */ -static unsigned int backlight_pwm_to_millipercent ( +STATIC_IFN_KUNIT +unsigned int backlight_pwm_to_millipercent(struct core_power *core_power,unsigned int pwm,unsigned int inst) {if (!is_vesa_abc (core_power, inst))
@@ -456,6 +458,8 @@ static unsigned int backlight_pwm_to_millipercent( core_power->bl_prop[inst].max_backlight_pwm); } +EXPORT_IF_KUNIT (backlight_pwm_to_millipercent); +static unsigned int backlight_pwm_to_millinit_legacy (struct core_power *core_power,unsigned int pwm,unsigned int inst) {
@@ -586,6 +590,8 @@ unsigned int backlight_millipercent_to_pwm( BACKLIGHT_MILLIPERCENT_MAX); } +EXPORT_IF_KUNIT (backlight_millipercent_to_pwm); +/* Legacy millinit→PWM conversion: scales linearly between * [min_brightness_millinits, max_brightness_millinits] → [min_backlight_pwm, max_backlight_pwm]. */
@@ -844,6 +850,8 @@ void initialize_backlight_caps(struct core_power *core_power, unsigned int inst) core_power->bl_prop[inst].backlight_caps_valid = true; } +EXPORT_IF_KUNIT (initialize_backlight_caps); +static void varibright_set_level (struct core_power *core_power) { if (!core_power->varibright_prop.varibright_active ||
drivers/gpu/drm/amd/display/modules/power/power_helpers.h +4 -0
@@ -201,6 +201,10 @@ void mod_power_set_backlight_control_type(struct core_power *core_power, unsigned int inst, enum backlight_control_type backlight_control_type);unsigned int backlight_millipercent_to_pwm (struct core_power *core_power,unsigned int millipercent,unsigned int inst); +#if IS_ENABLED (CONFIG_DRM_AMD_DC_KUNIT_TEST ) +unsigned int backlight_pwm_to_millipercent ( +struct core_power *core_power,unsigned int pwm,unsigned int inst); +#endif unsigned int backlight_millipercent_to_millinit (struct core_power *core_power,unsigned int millipercent,unsigned int inst); void fill_backlight_level_params(struct core_power *core_power,
Signature · 1 line
2.56.0
RFC PATCH 7/8 drm/amd/display: Map AUX brightness through the custom curve like PWM
In reply to Hugo Salesconvert_custom_brightness() rounds the user level to 8 bits and the luminance to a whole percent, then multiplies that percentage by the level again
Use the backlight_custom_curve_scale() helper from the PWM path instead. It places the level between user_min and user_max by the curve's luminance. This changes the AUX mapping on purpose. A level now asks for the luminance the curve gives it, as on PWM, and no longer for that luminance times the level
---
On the 1..512 nits fallback range with the GV301QH curve, the 512001 levels gave 182 different values from 1000 to 495968 millinits. Now they give 433295 different values from 1000 to 512000, and only levels 0 and 1 give 1000, instead of the bottom 6.1% of the scale. Half the scale moves from 159310 to 315265 millinits
The convert_custom_brightness() tests now check exact values through convert_brightness_from_user(), and dm_test_brightness_from_user_curve_min expects 261610 for half the AUX range now that the level goes in once. The early returns for DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE and for no data points keep the linear mapping. convert_brightness_to_user() never inverted the curve and doesn't change
As before, the AUX curve ends at 100% at signal 255. It doesn't read max_input_signal, which is a PWM limit. A point at signal 255 with less than 100% no longer holds the top level below max, as it already doesn't on PWM
KUnit: the four tests in patch 8, and the brightness values it adds to the luminance over 100% test, fail without this patch and pass with it
--- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 88 +----- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.h | 2 - .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 314 +++++++-------------- 3 files changed, 115 insertions(+), 289 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c +8 -80
@@ -117,18 +117,6 @@ int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps, }EXPORT_IF_KUNIT (get_brightness_range); -/* Rescale userspace [0..user_max] to the firmware curve's [0..255]. */ -static inline u32 scale_input_to_fw (int user_max,u64 input) -{ -return DIV_ROUND_CLOSEST_ULL (input *AMDGPU_MAX_BL_LEVEL , user_max); -} - -/* Rescale from [0..AMDGPU_MAX_BL_LEVEL] to [0..user_max] */ -static inline u32 scale_fw_to_input (int user_max,u64 input) -{ -return DIV_ROUND_CLOSEST_ULL (input * user_max,AMDGPU_MAX_BL_LEVEL ); -} -/* * Map user level 0..user_max to 0..output_max through the custom curve, * which ends at 100% at signal_max, or at AMDGPU_MAX_BL_LEVEL when the
@@ -198,71 +186,6 @@ u32 convert_brightness_to_millipercent(const struct amdgpu_dm_backlight_caps *caEXPORT_IF_KUNIT (convert_brightness_to_millipercent); -STATIC_IFN_KUNIT -void convert_custom_brightness(const struct amdgpu_dm_backlight_caps *caps, -unsigned int user_max,uint32_t *user_brightness) -{ -u32 brightness =scale_input_to_fw (user_max, *user_brightness); -u8 lower_signal, upper_signal, upper_lum, lower_lum, lum; -int left, right; - -if (amdgpu_dc_debug_mask &DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE ) -return ; - -if (!caps->data_points) -return ; - -/* -* Handle the case where brightness is below the first data point -* Interpolate between (0,0) and (first_signal, first_lum) -*/ -if (brightness < caps->luminance_data[0 ].input_signal) { - lum =DIV_ROUND_CLOSEST (caps->luminance_data[0 ].luminance * brightness, - caps->luminance_data[0 ].input_signal); - goto scale; - } - - left =0 ; - right = caps->data_points -1 ; -while (left <= right) { -int mid = left + (right - left) /2 ; -u8 signal = caps->luminance_data[mid].input_signal; - -/* Exact match found */ -if (signal == brightness) { - lum = caps->luminance_data[mid].luminance; - goto scale; - } - -if (signal < brightness) - left = mid +1 ; -else - right = mid -1 ; - } - -/* verify bound */ -if (left >= caps->data_points) - left = caps->data_points -1 ; - -/* At this point, left > right */ - lower_signal = caps->luminance_data[right].input_signal; - upper_signal = caps->luminance_data[left].input_signal; - lower_lum = caps->luminance_data[right].luminance; - upper_lum = caps->luminance_data[left].luminance; - -/* interpolate */ -if (right == left || !lower_lum) - lum = upper_lum; -else - lum = lower_lum +DIV_ROUND_CLOSEST ((upper_lum - lower_lum) * - (brightness - lower_signal), - upper_signal - lower_signal); -scale: - *user_brightness =scale_fw_to_input (user_max,DIV_ROUND_CLOSEST (lum * brightness,101 )); -} - -EXPORT_IF_KUNIT (convert_custom_brightness); -STATIC_IFN_KUNIT u32 convert_brightness_from_user(const struct amdgpu_dm_backlight_caps *caps,uint32_t brightness)
@@ -272,10 +195,15 @@ u32 convert_brightness_from_user(const struct amdgpu_dm_backlight_caps *caps,if (!get_brightness_range (caps, &user_min, &user_max))return brightness; -convert_custom_brightness (caps, user_max, &brightness); +if ((amdgpu_dc_debug_mask &DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE ) || + !caps->data_points) +return user_min +DIV_ROUND_CLOSEST_ULL ((u64)(user_max - user_min) * brightness, + user_max); -/* Rescale 0..user_max to user_min..user_max */ -return user_min +DIV_ROUND_CLOSEST_ULL ((u64)(user_max - user_min) * brightness, user_max); +/* max_input_signal is a PWM limit, so the AUX curve ends at signal 255 */ +return user_min +backlight_custom_curve_scale (caps, brightness, user_max, +AMDGPU_MAX_BL_LEVEL , + user_max - user_min); }EXPORT_IF_KUNIT (convert_brightness_from_user);
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.h +0 -2
@@ -67,8 +67,6 @@ ssize_t panel_power_savings_store(struct device *device, const char *buf, size_t count);int get_brightness_range (const struct amdgpu_dm_backlight_caps *caps,unsigned int *user_min,unsigned int *user_max); -void convert_custom_brightness (const struct amdgpu_dm_backlight_caps *caps, -unsigned int user_max,uint32_t *user_brightness);u32 convert_brightness_to_millipercent (const struct amdgpu_dm_backlight_caps *caps,u32 user_brightness); u32 convert_brightness_from_user(const struct amdgpu_dm_backlight_caps *caps,
drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c +107 -207
447 lines not on this page · raw message
Signature · 1 line
2.56.0
RFC PATCH 8/8 drm/amd/display: Add KUnit tests for the AUX custom brightness curve
In reply to Hugo SalesTest the AUX mapping with the GV301QH curve on the 1..512 nits fallback range. A request for 0 and for max must give user_min and user_max on both paths, and values must match ones worked out by hand. A walk over every request must never go down, must leave the minimum within 1% of the scale and must change the output on most steps. An AUX curve whose max_input_signal is below 255 must still run to 100% at signal 255
Check convert_brightness_from_user() on the PWM and AUX ranges in the test with a luminance over 100% too
---
KUnit on UML with PROVE_LOCKING: amdgpu_dm_backlight passes 83 of 83 without this patch and 87 of 87 with it. Without patch 7 the four new tests fail, and so do the brightness checks added to the luminance over 100% test
This and patch 7 are the AUX half of the series. My laptop's panel ignores AMD's AUX brightness, so I tested them only in KUnit and with the amdgpu_dm_brightness trace on a forced AUX path. Patches 1 to 6 don't need them
--- .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 133 ++++++++++++++++++++- 1 file changed, 132 insertions(+), 1 deletion(-)
drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c +132 -1
@@ -1517,6 +1517,75 @@ static void dm_test_brightness_curve_pwm_steps(struct kunit *test)KUNIT_EXPECT_GT (test, steps, (user_max - user_min) /2 ); } +/** +* dm_test_brightness_from_user_curve_floor - Test Custom curve ends are the minimum and max +* @test: The KUnit test context +* +* A request for 0 must reach the panel as the minimum, not as the minimum +* applied twice, and a request for max must reach max, on both the PWM and +* the AUX range +*/ +static void dm_test_brightness_from_user_curve_floor (struct kunit *test) +{ +struct amdgpu_dm_backlight_caps caps = {}; +unsigned int user_min, user_max; + +dm_test_enable_custom_curve (test); + +set_gv301qh_caps (&caps, false); +get_brightness_range (&caps, &user_min, &user_max); +KUNIT_EXPECT_EQ (test, user_min,3084U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,0 ), (u32 )user_min); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps, user_max), (u32 )user_max); + +set_gv301qh_caps (&caps, true); +get_brightness_range (&caps, &user_min, &user_max); +KUNIT_EXPECT_EQ (test, user_min,1000U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,0 ), (u32 )user_min); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps, user_max), (u32 )user_max); +} + +/** +* dm_test_brightness_from_user_curve_values - Test Custom curve output on the AUX range +* @test: The KUnit test context +* +* Worked out by hand on the AUX range of 1000..512000 millinits. 1000 is +* signal 0.498, so 0.0711% from (0, 0) to the first point (14, 2), and +* 1000 + 511000 * 0.000711 gives 1364. 200000 is signal 99.61, between +* (98, 50) and (103, 52), so 50.644% and 259790 +*/ +static void dm_test_brightness_from_user_curve_values (struct kunit *test) +{ +struct amdgpu_dm_backlight_caps caps = {}; + +dm_test_enable_custom_curve (test); +set_gv301qh_caps (&caps, true); + +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,1000 ),1364U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,200000 ),259790U ); +} + +/** +* dm_test_brightness_from_user_curve_aux_top - Test AUX curve ignores the PWM max_input_signal +* @test: The KUnit test context +* +* On AUX the curve runs to 100% at signal 255 even when max_input_signal, +* a PWM limit, is lower. With max_input_signal 251, 499953 is signal +* 249.00003, between the last point (247, 98) and 100% at 255, so 98.5% +* and 504335. Ending the curve at 251 would give 99% and 506890 +*/ +static void dm_test_brightness_from_user_curve_aux_top (struct kunit *test) +{ +struct amdgpu_dm_backlight_caps caps = {}; + +dm_test_enable_custom_curve (test); +set_gv301qh_caps (&caps, true); + caps.max_input_signal =251 ; + +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,499953 ),504335U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,512000 ),512000U ); +} +/** * dm_test_brightness_curve_pwm_points_above_max - Test PWM curve points above max_input_signal * @test: The KUnit test context
@@ -1543,6 +1612,51 @@ static void dm_test_brightness_curve_pwm_points_above_max(struct kunit *test)KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (&caps,0x101 *200 ),100000U ); } +static void check_curve_steps (struct kunit *test, +const struct amdgpu_dm_backlight_caps *caps) +{ +unsigned int user_min, user_max, user, steps =0 ; +u32 prev, cur; + +get_brightness_range (caps, &user_min, &user_max); + prev = user_min; +for (user =0 ; user <= user_max; user++) { + cur =convert_brightness_from_user (caps, user); +if (cur < prev || cur > user_max) { +KUNIT_FAIL (test,"%u gives %u after %u, range %u..%u" , + user, cur, prev, user_min, user_max); +return ; + } + steps += cur != prev; + prev = cur; + } + +KUNIT_EXPECT_GT (test,convert_brightness_from_user (caps, user_max /100 ), (u32 )user_min); +KUNIT_EXPECT_GT (test, steps, (user_max - user_min) /2 ); +} + +/** +* dm_test_brightness_from_user_curve_monotonic - Test Custom curve never drops and keeps its steps +* @test: The KUnit test context +* +* Every request from 0 to max, on both the PWM and the AUX range, must give +* a value in user_min..user_max that is no lower than the one for the request +* before. 1% of the range must already be above the minimum, and most +* requests must change the output +*/ +static void dm_test_brightness_from_user_curve_monotonic (struct kunit *test) +{ +struct amdgpu_dm_backlight_caps caps = {}; + +dm_test_enable_custom_curve (test); + +set_gv301qh_caps (&caps, false); +check_curve_steps (test, &caps); + +set_gv301qh_caps (&caps, true); +check_curve_steps (test, &caps); +} +/** * dm_test_brightness_curve_pwm_luminance_over_100 - Test PWM with an ATIF luminance over 100% * @test: The KUnit test context
@@ -1550,7 +1664,9 @@ static void dm_test_brightness_curve_pwm_points_above_max(struct kunit *test)* Points (100, 50), (200, 150) and (250, 120) claim more than 100%. The * curve uses them as given and caps only the result, as before. * Level 125 is 50 + 100 * 25 / 100 = 75%, and the curve reaches full -* brightness at level 150, halfway to the second point, and stays there +* brightness at level 150, halfway to the second point, and stays there. +* On the AUX range of 1..255 nits, level 125000 is signal 125, so 1000 + +* 75% of 254000 */ static void dm_test_brightness_curve_pwm_luminance_over_100 (struct kunit *test) {
@@ -1564,9 +1680,20 @@ static void dm_test_brightness_curve_pwm_luminance_over_100(struct kunit *test)dm_test_enable_custom_curve (test);set_custom_curve_caps (&caps, points,ARRAY_SIZE (points)); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,125 *0x101 ),49151U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,150 *0x101 ),65535U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,225 *0x101 ),65535U );KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (&caps,125 *0x101 ),75000U );KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (&caps,150 *0x101 ),100000U );KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (&caps,225 *0x101 ),100000U ); + + caps.aux_support = true; + caps.aux_min_input_signal =1 ; + caps.aux_max_input_signal =255 ; + +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,125000 ),191500U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,150000 ),255000U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,225000 ),255000U ); }/**
@@ -2268,7 +2395,11 @@ static struct kunit_case dm_backlight_test_cases[] = {KUNIT_CASE (dm_test_brightness_from_user_curve_min),KUNIT_CASE (dm_test_brightness_curve_pwm_values),KUNIT_CASE (dm_test_brightness_curve_pwm_steps), +KUNIT_CASE (dm_test_brightness_from_user_curve_floor), +KUNIT_CASE (dm_test_brightness_from_user_curve_values), +KUNIT_CASE (dm_test_brightness_from_user_curve_aux_top),KUNIT_CASE (dm_test_brightness_curve_pwm_points_above_max), +KUNIT_CASE (dm_test_brightness_from_user_curve_monotonic),KUNIT_CASE (dm_test_brightness_curve_pwm_luminance_over_100),KUNIT_CASE (dm_test_brightness_range_zero_signals), /* amdgpu_dm_backlight_fill_props */
Signature · 1 line
2.56.0
Hi Hugo,
Thanks for the series and for digging into this.
I have been working on a similar backlight update that takes a different approach: instead of fixing the conversion in amdgpu_dm, it removes it. DM passes a linear brightness percentage for every panel type, and the power module does all of the conversion:
- VESA AUX: nits, mapped linearly over the panel luminance range from EDID.
- AMD AUX: nits on OLED, PWM otherwise.
- PWM: through the power module LUT built from the ATIF curve.
With that, most of this series no longer applies, since the DM code it touches is gone.
Patches 2/8 and 3/8 (LUT interpolation in the power module) are still useful on top of it, and I have already given my Reviewed-by on those. Could you test it on your machine?
drm/amd/display: Use panel luminance range for AUX backlight in power module
drm/amd/display: Add power module API to set brightness percent
drm/amd/display: Pass linear brightness percent to power module
drm/amd/display: Read PWM back linearly with the linear backlight curve
Thanks,
Ray
Quoted 98 lines · Hugo Sales
Both backlight paths in
amdgpu_dm_backlight.cmap the 16-bit user level through the ATIF custom brightness curve, whose points pair an 8-bit signal with a whole-percent luminance. Both round the level to one of those signals and the luminance to a whole percent first. That hurts most at the dim end, where one percent is a large share of the light. With the curve of an ASUS ROG Flow X13 GV301QH, minimum 12 and 49 points:- On PWM, the lowest level asks for 2% of full brightness, and the first 2% of the scale steps from there to 3%, 4% and 5%. Each step is 50%, 33% and 25% brighter than the one before. The bottom 10% of the scale has 18 different brightness values
- On AUX, with amdgpu.backlight=1 to force the unsupported AUX path, the 1..512 nits fallback range stays at 1 nit for the bottom 6.1% of the scale, and the next steps go to 3, 5 and 7 nits. The bottom 10% of the scale has 4 different values, and the top stops at 496 nitsThis series interpolates on the unrounded level and rounds once at the end. On PWM, the power module also interpolates between its LUT entries now, from millipercent to PWM and back. The bottom 10% of the scale then has 6018 different values on PWM instead of 18, and 33311 on AUX instead of 4. Level 0 still gives the panel's minimum and the top level gives its maximum. On AUX only levels 0 and 1 now sit at the minimum, instead of the bottom 6.1% of the scale
- Patch 1 renames the min and max of the user range to
user_minanduser_max, to avoid confusion with the curve'ssignal_maxand theoutput_maxthat patch 4 adds. No functional change
- Patches 2 and 3 interpolate between the power module's LUT entries, from millipercent to PWM and back
- Patch 4 stops rounding the PWM path
- Patches 5 and 6 test the PWM path and the LUT with the GV301QH curve
- Patch 7 maps AUX through the same curve code as PWM, which reaches max at the top, and removesconvert_custom_brightness()
- Patch 8 tests the AUX pathPatch 7 changes what an AUX panel gets. Until now an AUX level asked for the curve's luminance percentage times the level, so the level went in twice. Now it asks for min plus that percentage of the range above min, the same luminance the PWM path asks the power module for. On the fallback range, half the scale goes from 159 nits to 315 nits. I found no reason for the old product, but I may be missing the intent. I also can't test it meaningfully, as I don't have a panel with an AUX backlight
The series applies on top of "drm/amd/display: Fix the doubled AUX backlight minimum on the custom curve" [1]. That series fixes the minimum going in twice on AUX, and asks whether the ATIF curve is meant for AUX at all. Patch 7 keeps applying the curve on AUX, as the code always has. Since it changes what AUX panels get, it has no Fixes: tags
One question for you: on PWM, above the last point the curve still runs to 100% at
max_input_signal, asbacklight_signal_to_luminance()did, while the level maps to the 0..255 signal scale over the whole user range. Withmax_input_signalbelow 255 the top of the PWM scale is flat at full brightness. AUX keeps ending the curve at 255, as it did before. Should PWM end at 255 too?This is also a request for testing. My laptop drives its panel over PWM, and the panel ignores AMD's AUX brightness, so I tested the AUX path only with KUnit and with the
amdgpu_dm_brightnesstrace on a forced AUX path. If you have a panel with an AUX backlight and an ATIF custom curve, a Tested-by on patches 7 and 8 would help. The middle of the scale should get brighter, the bottom should no longer sit at the minimum and the top should reach full brightness. Patches 1 to 6 don't depend on them, so they can go in without the AUX half[1] https://lore.kernel.org/r/[email protected]
Signed-off-by Hugo Sales <[email protected]>---
Hugo Sales (8):
drm/amd/display: Rename the user brightness range touser_minanduser_maxdrm/amd/display: Interpolate between backlight LUT entries drm/amd/display: Interpolate when reading PWM back as a percentage drm/amd/display: Stop rounding PWM brightness to whole percents drm/amd/display: Add KUnit tests for the custom brightness curve drm/amd/display: Add KUnit tests for backlight LUT interpolation drm/amd/display: Map AUX brightness through the custom curve like PWM drm/amd/display: Add KUnit tests for the AUX custom brightness curve.../amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 218 ++--- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.h | 6 +- .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 923 +++++++++++++++------ .../gpu/drm/amd/display/modules/power/power_abm.c | 72 +- .../drm/amd/display/modules/power/power_helpers.h | 4 + 5 files changed, 784 insertions(+), 439 deletions(-)---
base-commit:ba496bb837882eb1af58f41572fc289e46c77b8c
change-id: 20261102-backlight-curve-rfc-1bb4b6dee24c
prerequisite-message-id: <[email protected]>
prerequisite-patch-id:0eefd25bf4ad8f6d779c7a0b0a63f84969b67fca
prerequisite-patch-id:016515673f79f623de1e50dd513e5440587329b6
prerequisite-patch-id:ba453baf376b5410d66feacb1956eea933042ae8Best regards,
--
Hugo Sales <[email protected]>