drm/amd/display: Check the ATIF backlight curve
Subsystems (3)
cover drm/amd/display: Check the ATIF backlight curve
The ATIF custom brightness curve comes from firmware, and amdgpu uses it without checking that its points rise. Patch 1 checks the curve once at probe. It drops a curve whose signals fall, caps a luminance over 100% at 100%, and raises a luminance that falls. When it changes the curve, it warns about a firmware bug. Patch 2 adds KUnit tests. Patch 3 turns the debug message for an invalid signal range into a warning too, so users have something to report
I haven't seen a curve this would change. My laptop's curve passes, and so do 240 static curves I found in public ACPI dumps of 15 laptop models. On many Ryzen laptops the firmware builds the curve at boot, though, and a scan of the tables can't see those. The bug reports I found of an ASUS curve over 100% turned out to be a different overflow bug. Sashiko flagged a luminance over 100% while reviewing [1], and I extended the check to falling points. So this is hardening. If you'd rather not carry the check, I'm fine with dropping it
It applies on top of "drm/amd/display: Stop rounding backlight levels to whole percents" [1]. Patch 2's brightness checks use the curve mapping from that series
[1] https://lore.kernel.org/lkml/[email protected]/
---
Hugo Sales (3):
drm/amd/display: Repair or drop a bad ATIF backlight curve drm/amd/display: Add KUnit tests for the ATIF backlight curve check drm/amd/display: Warn about an invalid ATIF backlight signal range
.../amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 50 +++++- .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 184 +++++++++++++++++++++ 2 files changed, 232 insertions(+), 2 deletions(-)
---
base-commit: ba496bb837882eb1af58f41572fc289e46c77b8c
change-id: 20261103-backlight-curve-validate-243c4ce9692c
prerequisite-message-id: <[email protected]>
prerequisite-patch-id: 0eefd25bf4ad8f6d779c7a0b0a63f84969b67fca
prerequisite-patch-id: 016515673f79f623de1e50dd513e5440587329b6
prerequisite-patch-id: ba453baf376b5410d66feacb1956eea933042ae8
prerequisite-message-id: <[email protected]>
prerequisite-patch-id: ab08642e6487dac872fecb986cee493826c90ca9
prerequisite-patch-id: 615ae71fc87a132500083a791977cedffb5c5535
prerequisite-patch-id: d5f11b9531761fbfd4103cebc6f0e9403cd53772
prerequisite-patch-id: 95660ed7df12918c00809bf09d9d979c597bcbb6
prerequisite-patch-id: 4bf24c99ce3da811000e96e67ca6b93df79eb964
prerequisite-patch-id: 33257e757b334f4a8cb44b4efc24a3f3cd000307
prerequisite-patch-id: dec6b61e59750a963d14cb6905cb76a4cb76f77c
prerequisite-patch-id: f1fe27995f1a36d9382674eb9951ea5fe5f18a91
Best regards,
Signature · 1 line
Hugo Sales <[email protected]>
RFC PATCH 1/3 drm/amd/display: Repair or drop a bad ATIF backlight curve
The custom brightness curve code expects the input signals and luminances of the ATIF curve to increase monotonically. They come from firmware as u8 values, and nothing checks them
Check the curve once, in amdgpu_dm_validate_backlight_caps(), next to the signal range check. If a signal falls, drop the curve and map linearly. Cap a luminance over 100% at 100%, and raise a luminance that falls back to the one before it. When the check changes the curve, warn about a firmware bug
The curve code only caps its result at 100%. A point over 100% then makes the curve reach full brightness before that point and stay flat up to it. With the point capped, the curve reaches full brightness at the point itself
Dropping the curve also changes ABM. The power module rejected a curve whose signals fall and built its default quadratic LUT. With no points it builds a linear LUT over the firmware's signal range and uses the firmware's AC and DC levels, as it does when firmware reports no curve. The power module still rejects a capped or raised luminance, as before
---
The data_points clamp duplicates "drm/amdgpu: Clamp the ATIF backlight curve to its 99 points", which fixes the overflow where the ATIF reply is read:
https://lore.kernel.org/r/[email protected]
This check stays correct whichever of the two lands first
The check keeps a repeated signal, which the mapping steps over as before. It sits in amdgpu_dm rather than in amdgpu_atif_query_backlight_caps() so KUnit on UML, which has no ACPI, can reach it
--- .../amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 44 ++++++++++++++++++++++ 1 file changed, 44 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c +44 -0
@@ -52,6 +52,48 @@#include "dm_helpers.h" #if defined(CONFIG_ACPI ) ||IS_ENABLED (CONFIG_DRM_AMD_DC_KUNIT_TEST ) +/* +* The brightness mapping needs signals that never fall from point to point +* and luminance that never falls or passes 100%. Drop a curve whose +* signals fall, and flatten the luminance of the others +*/ +static void backlight_validate_custom_curve (struct amdgpu_display_manager *dm, +struct amdgpu_dm_backlight_caps *caps) +{ +struct amdgpu_dm_luminance_data *p = caps->luminance_data; +int over =0 , falling =0 ; +int i; + +if (caps->data_points >ARRAY_SIZE (caps->luminance_data)) + caps->data_points =ARRAY_SIZE (caps->luminance_data); + +for (i =1 ; i < caps->data_points; i++) { +if (p[i].input_signal < p[i -1 ].input_signal) { +drm_warn (adev_to_drm (dm->adev),FW_BUG +"ATIF: backlight curve signal %u at point %d is below %u, using a linear curve\n" , + p[i].input_signal, i, p[i -1 ].input_signal); + caps->data_points =0 ; +return ; + } + } + +for (i =0 ; i < caps->data_points; i++) { +if (p[i].luminance >100 ) { + p[i].luminance =100 ; + over++; + } +if (i && p[i].luminance < p[i -1 ].luminance) { + p[i].luminance = p[i -1 ].luminance; + falling++; + } + } + +if (over || falling) +drm_warn (adev_to_drm (dm->adev),FW_BUG +"ATIF: backlight curve has %d points over 100%%, set to 100%%, and %d falling points, raised to the point before\n" , + over, falling); +} +STATIC_IFN_KUNIT void amdgpu_dm_validate_backlight_caps(struct amdgpu_display_manager *dm, int bl_idx) {
@@ -66,6 +108,8 @@ void amdgpu_dm_validate_backlight_caps(struct amdgpu_display_manager *dm, int bl caps->min_input_signal, caps->max_input_signal); caps->caps_valid = false; } + +backlight_validate_custom_curve (dm, caps); }EXPORT_IF_KUNIT (amdgpu_dm_validate_backlight_caps);#endif
Signature · 1 line
2.56.0
RFC PATCH 2/3 drm/amd/display: Add KUnit tests for the ATIF backlight curve check
In reply to Hugo SalesTest the curve check in amdgpu_dm_validate_backlight_caps():
- the GV301QH curve stays as it is
- a luminance over 100% becomes 100%
- a falling luminance rises to the point before
- a curve whose signals fall gets dropped
- a repeated signal stays
- a curve with more points than luminance_data holds is cut to fit
Also check the brightness the PWM and AUX paths give on the checked curves
---
KUnit: dm_test_validate_custom_curve_unsorted and the luminance checks in the falling test fail without patch 1
--- .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 184 +++++++++++++++++++++ 1 file changed, 184 insertions(+)
drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c +184 -0
@@ -1696,6 +1696,185 @@ static void dm_test_brightness_curve_pwm_luminance_over_100(struct kunit *test)KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,225000 ),255000U ); } +/* Tests for the custom curve check in amdgpu_dm_validate_backlight_caps() */ + +static struct amdgpu_display_manager *alloc_validate_dm (struct kunit *test) +{ +struct amdgpu_display_manager *dm =dm_kunit_alloc_dm (test); + + dm->adev =dm_kunit_alloc_adev (test); +/* drm_warn() prints the curve warnings against this device */ +KUNIT_ASSERT_NOT_NULL (test,adev_to_drm (dm->adev)->dev); + +return dm; +} + +static struct amdgpu_dm_backlight_caps * +validate_custom_curve (struct amdgpu_display_manager *dm, +const struct amdgpu_dm_luminance_data *points,int n) +{ +struct amdgpu_dm_backlight_caps *caps = &dm->backlight_caps[0 ]; + +set_custom_curve_caps (caps, points, n); + caps->caps_valid = true; +amdgpu_dm_validate_backlight_caps (dm,0 ); + +return caps; +} + +/** +* dm_test_validate_custom_curve_gv301qh - Test a good ATIF curve is left alone +* @test: The KUnit test context +* +* The GV301QH curve rises and stays within 100%, so the check must keep +* every point as it is +*/ +static void dm_test_validate_custom_curve_gv301qh (struct kunit *test) +{ +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps = &dm->backlight_caps[0 ]; + +set_gv301qh_caps (caps, false); + caps->caps_valid = true; +amdgpu_dm_validate_backlight_caps (dm,0 ); + +KUNIT_EXPECT_TRUE (test, caps->caps_valid); +KUNIT_ASSERT_EQ (test, caps->data_points,ARRAY_SIZE (gv301qh_curve)); +KUNIT_EXPECT_MEMEQ (test, caps->luminance_data, gv301qh_curve,sizeof (gv301qh_curve)); +} + +/** +* dm_test_validate_custom_curve_luminance_over_100 - Test ATIF luminance over 100% becomes 100% +* @test: The KUnit test context +* +* Points (100, 50), (200, 150) and (250, 120) become (100, 50), (200, 100) +* and (250, 100). Level 150 is then 50 + 50 * 50 / 100 = 75% and level 200 +* is full brightness. Over AUX, 301176 of 512000 is just below signal 150, +* so it lands one below 75% of 1000..512000, at 384249 +*/ +static void dm_test_validate_custom_curve_luminance_over_100 (struct kunit *test) +{ +static const struct amdgpu_dm_luminance_data points[] = { + { .input_signal =100 , .luminance =50 }, + { .input_signal =200 , .luminance =150 }, + { .input_signal =250 , .luminance =120 }, + }; +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps; + +dm_test_enable_custom_curve (test); + caps =validate_custom_curve (dm, points,ARRAY_SIZE (points)); + +KUNIT_ASSERT_EQ (test, caps->data_points,3 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[0 ].luminance,50 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[1 ].luminance,100 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[2 ].luminance,100 ); + +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,150 *0x101 ),49151U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,200 *0x101 ),65535U ); +KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (caps,150 *0x101 ),75000U ); +KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (caps,200 *0x101 ),100000U ); + + caps->aux_support = true; + caps->aux_min_input_signal =1 ; + caps->aux_max_input_signal =512 ; +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,301176 ),384249U ); +} + +/** +* dm_test_validate_custom_curve_falling - Test a falling ATIF point is raised +* @test: The KUnit test context +* +* In (50, 30), (100, 60), (150, 40) and (200, 80), the point at 150 is +* raised to 60%, so the curve is flat from 100 to 150. Level 125 is 60% on +* both paths, which over AUX is 307600 in 1000..512000 +*/ +static void dm_test_validate_custom_curve_falling (struct kunit *test) +{ +static const struct amdgpu_dm_luminance_data points[] = { + { .input_signal =50 , .luminance =30 }, + { .input_signal =100 , .luminance =60 }, + { .input_signal =150 , .luminance =40 }, + { .input_signal =200 , .luminance =80 }, + }; +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps; + +dm_test_enable_custom_curve (test); + caps =validate_custom_curve (dm, points,ARRAY_SIZE (points)); + +KUNIT_ASSERT_EQ (test, caps->data_points,4 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[1 ].luminance,60 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[2 ].luminance,60 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[3 ].luminance,80 ); + +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,125 *0x101 ),39321U ); +KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (caps,125 *0x101 ),60000U ); + + caps->aux_support = true; + caps->aux_min_input_signal =1 ; + caps->aux_max_input_signal =512 ; +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,250980 ),307600U ); +} + +/** +* dm_test_validate_custom_curve_unsorted - Test a curve whose signals fall is dropped +* @test: The KUnit test context +* +* A signal below the one before leaves no curve, so the mapping is +* linear. A repeated signal is a step: the curve stays, and level 100 is +* at the later point's 60% +*/ +static void dm_test_validate_custom_curve_unsorted (struct kunit *test) +{ +static const struct amdgpu_dm_luminance_data falling[] = { + { .input_signal =100 , .luminance =40 }, + { .input_signal =80 , .luminance =60 }, + }; +static const struct amdgpu_dm_luminance_data repeated[] = { + { .input_signal =50 , .luminance =20 }, + { .input_signal =100 , .luminance =40 }, + { .input_signal =100 , .luminance =60 }, + }; +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps; + +dm_test_enable_custom_curve (test); + + caps =validate_custom_curve (dm, falling,ARRAY_SIZE (falling)); +KUNIT_EXPECT_EQ (test, caps->data_points,0 ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,32768 ),32768U ); + + caps =validate_custom_curve (dm, repeated,ARRAY_SIZE (repeated)); +KUNIT_EXPECT_EQ (test, caps->data_points,3 ); +KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (caps,100 *0x101 ),60000U ); +} + +/** +* dm_test_validate_custom_curve_too_many_points - Test data_points is cut to the array +* @test: The KUnit test context +* +* data_points is a u8 from firmware, so it can claim more points than +* luminance_data holds. The check keeps the points that fit +*/ +static void dm_test_validate_custom_curve_too_many_points (struct kunit *test) +{ +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps = &dm->backlight_caps[0 ]; +int i; + +set_gv301qh_caps (caps, false); +for (i =0 ; i <ARRAY_SIZE (caps->luminance_data); i++) { + caps->luminance_data[i].input_signal =2 * i; + caps->luminance_data[i].luminance = i; + } + caps->data_points =255 ; + caps->caps_valid = true; +amdgpu_dm_validate_backlight_caps (dm,0 ); + +KUNIT_EXPECT_EQ (test, caps->data_points,ARRAY_SIZE (caps->luminance_data)); +} +/** * dm_test_brightness_range_zero_signals - Test Brightness range with zero min and max signals * @test: The KUnit test context
@@ -2365,6 +2544,11 @@ static struct kunit_case dm_backlight_test_cases[] = {KUNIT_CASE (dm_test_backlight_caps_aux_support_noop),/* amdgpu_dm_validate_backlight_caps */ KUNIT_CASE_PARAM (dm_test_validate_backlight_caps, dm_validate_caps_gen_params), +KUNIT_CASE (dm_test_validate_custom_curve_gv301qh), +KUNIT_CASE (dm_test_validate_custom_curve_luminance_over_100), +KUNIT_CASE (dm_test_validate_custom_curve_falling), +KUNIT_CASE (dm_test_validate_custom_curve_unsorted), +KUNIT_CASE (dm_test_validate_custom_curve_too_many_points),/* get_brightness_range */ KUNIT_CASE (dm_test_brightness_range_null_caps), KUNIT_CASE(dm_test_brightness_range_pwm),
Signature · 1 line
2.56.0
Re: [PATCH RFC 2/3] drm/amd/display: Add KUnit tests for the ATIF backlight curve check
Hi Hugo,
Thanks for this.
With the backlight update in the other thread, amdgpu_dm no longer maps brightness through the ATIF curve; only the power module uses it, and it validates the curve itself. So 1/3 and 2/3 are no longer needed. I have replied to 3/3 with my Reviewed-by.
Thanks,
Ray
Quoted 223 lines · Hugo Sales
Test the curve check in
amdgpu_dm_validate_backlight_caps():- the GV301QH curve stays as it is
- a luminance over 100% becomes 100%
- a falling luminance rises to the point before
- a curve whose signals fall gets dropped
- a repeated signal stays
- a curve with more points thanluminance_dataholds is cut to fitAlso check the brightness the PWM and AUX paths give on the checked curves
Signed-off-by Hugo Sales <[email protected]>---
KUnit:dm_test_validate_custom_curve_unsortedand the luminance checks in the falling test fail without patch 1--- .../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 184 +++++++++++++++++++++ 1 file changed, 184 insertions(+)drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c +184
-0@@ -1696,6 +1696,185 @@ static void dm_test_brightness_curve_pwm_luminance_over_100(struct kunit *test)KUNIT_EXPECT_EQ (test,convert_brightness_from_user (&caps,225000 ),255000U ); } +/* Tests for the custom curve check in amdgpu_dm_validate_backlight_caps() */ + +static struct amdgpu_display_manager *alloc_validate_dm (struct kunit *test) +{ +struct amdgpu_display_manager *dm =dm_kunit_alloc_dm (test); + + dm->adev =dm_kunit_alloc_adev (test); +/* drm_warn() prints the curve warnings against this device */ +KUNIT_ASSERT_NOT_NULL (test,adev_to_drm (dm->adev)->dev); + +return dm; +} + +static struct amdgpu_dm_backlight_caps * +validate_custom_curve (struct amdgpu_display_manager *dm, +const struct amdgpu_dm_luminance_data *points,int n) +{ +struct amdgpu_dm_backlight_caps *caps = &dm->backlight_caps[0 ]; + +set_custom_curve_caps (caps, points, n); + caps->caps_valid = true; +amdgpu_dm_validate_backlight_caps (dm,0 ); + +return caps; +} + +/** +* dm_test_validate_custom_curve_gv301qh - Test a good ATIF curve is left alone +* @test: The KUnit test context +* +* The GV301QH curve rises and stays within 100%, so the check must keep +* every point as it is +*/ +static void dm_test_validate_custom_curve_gv301qh (struct kunit *test) +{ +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps = &dm->backlight_caps[0 ]; + +set_gv301qh_caps (caps, false); + caps->caps_valid = true; +amdgpu_dm_validate_backlight_caps (dm,0 ); + +KUNIT_EXPECT_TRUE (test, caps->caps_valid); +KUNIT_ASSERT_EQ (test, caps->data_points,ARRAY_SIZE (gv301qh_curve)); +KUNIT_EXPECT_MEMEQ (test, caps->luminance_data, gv301qh_curve,sizeof (gv301qh_curve)); +} + +/** +* dm_test_validate_custom_curve_luminance_over_100 - Test ATIF luminance over 100% becomes 100% +* @test: The KUnit test context +* +* Points (100, 50), (200, 150) and (250, 120) become (100, 50), (200, 100) +* and (250, 100). Level 150 is then 50 + 50 * 50 / 100 = 75% and level 200 +* is full brightness. Over AUX, 301176 of 512000 is just below signal 150, +* so it lands one below 75% of 1000..512000, at 384249 +*/ +static void dm_test_validate_custom_curve_luminance_over_100 (struct kunit *test) +{ +static const struct amdgpu_dm_luminance_data points[] = { + { .input_signal =100 , .luminance =50 }, + { .input_signal =200 , .luminance =150 }, + { .input_signal =250 , .luminance =120 }, + }; +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps; + +dm_test_enable_custom_curve (test); + caps =validate_custom_curve (dm, points,ARRAY_SIZE (points)); + +KUNIT_ASSERT_EQ (test, caps->data_points,3 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[0 ].luminance,50 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[1 ].luminance,100 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[2 ].luminance,100 ); + +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,150 *0x101 ),49151U ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,200 *0x101 ),65535U ); +KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (caps,150 *0x101 ),75000U ); +KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (caps,200 *0x101 ),100000U ); + + caps->aux_support = true; + caps->aux_min_input_signal =1 ; + caps->aux_max_input_signal =512 ; +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,301176 ),384249U ); +} + +/** +* dm_test_validate_custom_curve_falling - Test a falling ATIF point is raised +* @test: The KUnit test context +* +* In (50, 30), (100, 60), (150, 40) and (200, 80), the point at 150 is +* raised to 60%, so the curve is flat from 100 to 150. Level 125 is 60% on +* both paths, which over AUX is 307600 in 1000..512000 +*/ +static void dm_test_validate_custom_curve_falling (struct kunit *test) +{ +static const struct amdgpu_dm_luminance_data points[] = { + { .input_signal =50 , .luminance =30 }, + { .input_signal =100 , .luminance =60 }, + { .input_signal =150 , .luminance =40 }, + { .input_signal =200 , .luminance =80 }, + }; +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps; + +dm_test_enable_custom_curve (test); + caps =validate_custom_curve (dm, points,ARRAY_SIZE (points)); + +KUNIT_ASSERT_EQ (test, caps->data_points,4 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[1 ].luminance,60 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[2 ].luminance,60 ); +KUNIT_EXPECT_EQ (test, caps->luminance_data[3 ].luminance,80 ); + +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,125 *0x101 ),39321U ); +KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (caps,125 *0x101 ),60000U ); + + caps->aux_support = true; + caps->aux_min_input_signal =1 ; + caps->aux_max_input_signal =512 ; +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,250980 ),307600U ); +} + +/** +* dm_test_validate_custom_curve_unsorted - Test a curve whose signals fall is dropped +* @test: The KUnit test context +* +* A signal below the one before leaves no curve, so the mapping is +* linear. A repeated signal is a step: the curve stays, and level 100 is +* at the later point's 60% +*/ +static void dm_test_validate_custom_curve_unsorted (struct kunit *test) +{ +static const struct amdgpu_dm_luminance_data falling[] = { + { .input_signal =100 , .luminance =40 }, + { .input_signal =80 , .luminance =60 }, + }; +static const struct amdgpu_dm_luminance_data repeated[] = { + { .input_signal =50 , .luminance =20 }, + { .input_signal =100 , .luminance =40 }, + { .input_signal =100 , .luminance =60 }, + }; +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps; + +dm_test_enable_custom_curve (test); + + caps =validate_custom_curve (dm, falling,ARRAY_SIZE (falling)); +KUNIT_EXPECT_EQ (test, caps->data_points,0 ); +KUNIT_EXPECT_EQ (test,convert_brightness_from_user (caps,32768 ),32768U ); + + caps =validate_custom_curve (dm, repeated,ARRAY_SIZE (repeated)); +KUNIT_EXPECT_EQ (test, caps->data_points,3 ); +KUNIT_EXPECT_EQ (test,convert_brightness_to_millipercent (caps,100 *0x101 ),60000U ); +} + +/** +* dm_test_validate_custom_curve_too_many_points - Test data_points is cut to the array +* @test: The KUnit test context +* +* data_points is a u8 from firmware, so it can claim more points than +* luminance_data holds. The check keeps the points that fit +*/ +static void dm_test_validate_custom_curve_too_many_points (struct kunit *test) +{ +struct amdgpu_display_manager *dm =alloc_validate_dm (test); +struct amdgpu_dm_backlight_caps *caps = &dm->backlight_caps[0 ]; +int i; + +set_gv301qh_caps (caps, false); +for (i =0 ; i <ARRAY_SIZE (caps->luminance_data); i++) { + caps->luminance_data[i].input_signal =2 * i; + caps->luminance_data[i].luminance = i; + } + caps->data_points =255 ; + caps->caps_valid = true; +amdgpu_dm_validate_backlight_caps (dm,0 ); + +KUNIT_EXPECT_EQ (test, caps->data_points,ARRAY_SIZE (caps->luminance_data)); +} +/** * dm_test_brightness_range_zero_signals - Test Brightness range with zero min and max signals * @test: The KUnit test context @@ -2365,6 +2544,11 @@ static struct kunit_case dm_backlight_test_cases[] = {KUNIT_CASE (dm_test_backlight_caps_aux_support_noop),/* amdgpu_dm_validate_backlight_caps */ KUNIT_CASE_PARAM (dm_test_validate_backlight_caps, dm_validate_caps_gen_params), +KUNIT_CASE (dm_test_validate_custom_curve_gv301qh), +KUNIT_CASE (dm_test_validate_custom_curve_luminance_over_100), +KUNIT_CASE (dm_test_validate_custom_curve_falling), +KUNIT_CASE (dm_test_validate_custom_curve_unsorted), +KUNIT_CASE (dm_test_validate_custom_curve_too_many_points),/* get_brightness_range */ KUNIT_CASE (dm_test_brightness_range_null_caps), KUNIT_CASE(dm_test_brightness_range_pwm),
RFC PATCH 3/3 drm/amd/display: Warn about an invalid ATIF backlight signal range
In reply to Hugo SalesWhen the ATIF signal range is out of bounds or narrower than AMDGPU_DM_MIN_SPREAD, amdgpu_dm_validate_backlight_caps() drops the firmware caps and the driver falls back to a range of 12-255. It only says so with drm_dbg_kms(), so a user whose brightness range changed finds nothing in dmesg to report
Make it a drm_warn() with FW_BUG, like the custom curve warnings
--- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c +4 -2
@@ -104,8 +104,10 @@ void amdgpu_dm_validate_backlight_caps(struct amdgpu_display_manager *dm, int bl caps->min_input_signal < 0 || spread > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT || spread < AMDGPU_DM_MIN_SPREAD) { -drm_dbg_kms (adev_to_drm (dm->adev),"DM: Invalid backlight caps: min=%d, max=%d\n" , - caps->min_input_signal, caps->max_input_signal); +drm_warn (adev_to_drm (dm->adev),FW_BUG +"ATIF: backlight signal range %d to %d is invalid, using %d to %d\n" , + caps->min_input_signal, caps->max_input_signal, +AMDGPU_DM_DEFAULT_MIN_BACKLIGHT ,AMDGPU_DM_DEFAULT_MAX_BACKLIGHT ); caps->caps_valid = false; }
Signature · 1 line
2.56.0
Re: [PATCH RFC 3/3] drm/amd/display: Warn about an invalid ATIF backlight signal range
Quoted 31 lines · Hugo Sales
When the ATIF signal range is out of bounds or narrower than
AMDGPU_DM_MIN_SPREAD,amdgpu_dm_validate_backlight_caps()drops the firmware caps and the driver falls back to a range of 12-255. It only says so withdrm_dbg_kms(), so a user whose brightness range changed finds nothing in dmesg to reportMake it a
drm_warn()withFW_BUG, like the custom curve warningsSigned-off-by Hugo Sales <[email protected]>--- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c +4
-2@@ -104,8 +104,10 @@ void amdgpu_dm_validate_backlight_caps(struct amdgpu_display_manager *dm, int bl caps->min_input_signal < 0 || spread > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT || spread < AMDGPU_DM_MIN_SPREAD) { -drm_dbg_kms (adev_to_drm (dm->adev),"DM: Invalid backlight caps: min=%d, max=%d\n" , - caps->min_input_signal, caps->max_input_signal); +drm_warn (adev_to_drm (dm->adev),FW_BUG +"ATIF: backlight signal range %d to %d is invalid, using %d to %d\n" , + caps->min_input_signal, caps->max_input_signal, +AMDGPU_DM_DEFAULT_MIN_BACKLIGHT ,AMDGPU_DM_DEFAULT_MAX_BACKLIGHT ); caps->caps_valid = false; }