On Mon, Jan 13, 2020 at 12:20:10PM +0100, Patrick Wildt wrote:
> The problem is that the last two values are 67 and 100.  If you go
> 5% down, it's 95.  The nearest will still be 100.  The code then
> realizes that it's the same level as before, and does nlevel--.
> But nlevel-- is 99, and not 67, because nlevel is the value and
> not the index of the bcl array.  So in essence the change needed
> is to decrease the index, not the value, and then look up the value.
That's what happens, but the problem isn't that my machine is lacking
levels between 67 and 100.

Rather, acpivout_find_brightness() seems to linear scale in levels
(which presumably is the case on newer machines), but my machine's
levels scale exponentially.

Diff below comments on this and hilights the two relevant checks:

Given an initial brightness of 100% (the last level), pressing the
function key to decrease the level eventually calls
acpivout_find_brightness() with `level = 100 + (-1 * 5) = 95', it then
iterates over the levels starting with the smallest.

Reaching the second last level (67 on my machine), the loop's body does

        mid = 67 + (100 - 67) / 2 = 83;
        if (67 <= 95 && 95 <= 83)
                return 67;
        if (83 <= 95 and 95 <= 100)
                return 100;

So for requesting level below 100 it always returns 100 itself because
the next level has a value of 67 which is way out of the 5% threshold
this algorithm assumes.

I suppose a better heuristic is required to make both linearly and
exponentially scaling machines happy, but this is currently too finicky
for me so I just reverted 1.14 in my tree.

Index: acpivout.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpivout.c,v
retrieving revision 1.16
diff -u -p -r1.16 acpivout.c
--- acpivout.c  14 Dec 2019 10:57:48 -0000      1.16
+++ acpivout.c  14 Jan 2020 14:19:08 -0000
@@ -218,6 +218,11 @@ acpivout_find_brightness(struct acpivout
 
        for (i = 0; i < sc->sc_bcl_len - 1; i++) {
                mid = sc->sc_bcl[i] + (sc->sc_bcl[i + 1] - sc->sc_bcl[i]) / 2;
+               /*
+                * XXX these checks assume levels to be on a linear scale,
+                * but some hardware provides exponentially scaled brightness
+                * levels (ThinkPad X230, T420).
+                */
                if (sc->sc_bcl[i] <= level && level <=  mid)
                        return sc->sc_bcl[i];
                if  (mid < level && level <= sc->sc_bcl[i + 1])

Reply via email to