search

openDsh / dash

Join us on Slack! https://join.slack.com/t/opendsh/shared_invite/zt-la398uly-a6eMH5ttEQhbtE6asVKx4Q
GNU General Public License v3.0
238 stars 69 forks source link

Raspberry Pi 7" display brightness fix #166

Open breakingspell opened 1 month ago

breakingspell commented 1 month ago

Brightness path:

Raspberry Pi's official DSI-based display will present itself differently when the unit is using the vc4-kms-v3d driver. This breaks the backlight level control provided with the dash brightness plugin. To replicate, set in /boot/firmware/config.txt and observe path:

dtoverlay=vc4-fkms-v3d - /sys/class/backlight/rpi_backlight/brightness dtoverlay=vc4-kms-v3d - /sys/class/backlight/10-0045/brightness

This PR fixes the path by checking for one and using the other if it doesn't exist.

It's worth noting that the DSI display seems to have trouble with the kms driver unless dtoverlay=vc4-kms-dsi-7inch is also loaded. I use the fkms fallback and it seems to works fine without hardware encoding. Also worth noting the existing RPi udev brightness permission works for both cases (/sys/class/backlight/%k/brightness)

Brightness range:

I noticed that the display's brightness range is rather odd, the brightest value is actually 200 while maximum value 255 is slightly dimmer. Plus, steps are much more pronounced between 0-100 as they are between 100-200. Someone made a comment about it ages ago, and about the energy draw as well.

I adjusted the min and max values from 76-255 to 20-180 (imperceptible from 200) and the step change to 20 units. However, I realize after posting the commit that this particular range affects all brightness operations using Session::System::Brightness, not just for the specific RPi display. Posting for review regardless, I may separate the range fix into it's own PR while seeing if it can be narrowed to only this panel.

Related issue (if applicable): fixes # https://github.com/openDsh/dash/issues/149

rsjudka commented 1 month ago

ill test out with the new brightness range using the mocked plugin (essentially just adjusts opacity) but from an initial glance i think that may cause an issue

it might be easy enough to get the min and max from the brightness module.. maybe?

breakingspell commented 1 month ago

The range oddity is specific to that RPi first-party panel (mine is manufactured by element14), so the new range may break the mock plugin and every other display, whoops :)

I'm thinking it may be possible to identify these specific panels and override the range only then.

breakingspell commented 1 month ago

Had the idea to possibly relocate the brightness range/step integers from arbiter.cpp to the brightness plugins themselves (mocked, official_rpi, x). Each supported display type would define their ranges there, while session.cpp would only call for a unit step up or down to set brightness.

This would be cleaner than identifying hardware and overriding the values in the session/arbiter to accommodate for one odd panel model.

nakato commented 1 week ago

it might be easy enough to get the min and max from the brightness module.. maybe?

Looks like it's hard-coded currently. https://github.com/raspberrypi/linux/blob/da87f91ad8450ccc5274cd7b6ba8d823b396c96f/drivers/regulator/rpi-panel-attiny-regulator.c#L340

If some panels are brightest at 255 and others 200, it would be worthwhile changing the driver in the kernel to get the max-brightness from the DeviceTree so it can be configured based on the hardware in use, and then the sysfs max-brightness value would reflect that.