lights: Adjust slightly off brightness conversion for maximum accuracy.

[MarijnS95]

This change fixes a slight mistake that was introduced when trying to
improve accuracy.
6199c66
That patch aims to ensure maximum brightness is reached - which indeed
happens for value 0xff - but adds a small discrepancy as outlined below.

Consider the following example:

bits = 10
sbits = bits - 8
maxval = (1 << bits) - 1

for brightness in range(256):
    print(f'{brightness} => '
          f'original: {brightness << sbits | brightness >> sbits}, '
          f'proper: {round(brightness * maxval / 255)}, '
          f'fixed: {brightness << sbits | brightness >> (8 - sbits)}')

When running the example (see output below) a consistent (yet small)
bump can be observed because the highest bits are replicated in the
lower bits of the left-shifted value. This results an overlap between
the lowest and highest bits instead of filling just the zeroes that were
introduced by the left shift.
This also means that going over the limit of 4 extra bits (so 12 bits in
total) results in a too large right shift, making it unable to reach
maximum brightness.
The "fixed" value in the rightmost column solves both issues by making
sure that only the most significant bits end up in the lowest bits.

However, this only works for values up to 16 bits. While it is not
likely to see anything higher than 12, better be safe than
sorry. Adjusting brightness is not a hot code path by any means so we
can justify an integer division ;)

Sample output:

224 => original: 952, proper: 899, fixed: 899
225 => original: 956, proper: 903, fixed: 903
226 => original: 952, proper: 907, fixed: 907
227 => original: 956, proper: 911, fixed: 911
228 => original: 953, proper: 915, fixed: 915
229 => original: 957, proper: 919, fixed: 919
230 => original: 953, proper: 923, fixed: 923
231 => original: 957, proper: 927, fixed: 927
232 => original: 954, proper: 931, fixed: 931
233 => original: 958, proper: 935, fixed: 935
234 => original: 954, proper: 939, fixed: 939
235 => original: 958, proper: 943, fixed: 943
236 => original: 955, proper: 947, fixed: 947
237 => original: 959, proper: 951, fixed: 951
238 => original: 955, proper: 955, fixed: 955
239 => original: 959, proper: 959, fixed: 959

Edit:
Before you comment this: Yes, I'm aware that using round is not accurate as the / 255 which truncates. Rounding up (eg. (brightness * max + 254) / 25) isn't accurate either, so I've pushed a + 254 / 2 (which is 127) that gives a perfectly rounded value.

[kholk]

The previous solution wanted to be a ninja stunt to do the calculation with a minimalistic approach on the required CPU cycles to come to a result between an acceptable error range.

It's also true that the devices in the current era have got nice computational power, so I guess that we can waste more cycles in favor of accuracy?

[MarijnS95]

@kholk We can still use a slightly-less minimalistic ninja stunt with the extra subtraction that works fine up to 16-bit brightness values: brightness << sbits | brightness >> (8 - sbits), as long as we de not expect values higher than that 😁

[kholk]

We don't expect anything higher than that for at least the next 7 years...

[MarijnS95]

@kholk Your choice... Fancy "bit-level hacking" or proper mult/div?

[kholk]

As long as the accuracy is good, I prefer the fancy hacking, as it would waste a lot less cycles... This thing will still be called PLENTY of times (for example, everytime the auto brightness has to scale of like 20, it'll set the brightness at least 19 times in 0.5 seconds...)

[MarijnS95]

Allright...

/me thinks about running basically anything else such as the Java apps and whatnot, eating much, much more cycles anyway... 😁

But then I love premature micro-optimizations :D

[kholk]

Yeah you're right about this, but if everyone never micro optimized anything... Then we'd need an EPYC to run Android.... :P

[MarijnS95]

I still have my doubts, sacrificing correctness and readability for negligible CPU cycles. But whatever, remember that discussion we had recently on FPC HAL, specifically the conversion from timeout-ioctl to indefinite-polling? There I converted the code that would wake up in kernel space every second, do some checks in userspace and go back to sleep for another second in the kernel to something that never wakes up at all, until something happens. It is now blocking indefinitely, waiting for either of two waitqueues (one triggered by the IRQ, the other coming from the eventfd that is written to from the system thread when execution state needs to change).

To say it differently, we have some extra CPU-cycle budget now 😁

Anyway, I have reintroduced the bit-level implementation, corrected as mentioned above. Together with that I have removed the useless mDevice pointer, that shaves off some ldr instructions. And the clz should surely tickle your fancy 😜
For completeness the mul-div implementation is left in place, and used whenever max_brightness isn't equal to 2^x - 1 or x is not in [8,16) (though, when would this ever happen??).