-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow for backlight dimming on supported displays #687
Comments
We are still coming back from gophercon.eu but I'm pinging @aykevl and @deadprogram for the interface design and breaking changes part, so they could review this. I'm in for the breaking changes, but in this case I think it will break too much, it's too common. |
The backlight works in different ways on different systems. On most systems, there is a single pin and high means on (and low means off). But there's also the PineTime, which has 3 backlight pins for different levels of brightness and they actually work in reverse: low is on and high is off. My proposal would be to remove backlight support from the display drivers, or at least allow Alternative: I should also point out that humans have logarithmic perception of brightness, so if we set the backlight to 50% PWM value it actually appears much brighter (75% or so, I don't know the actual number). Changes in brightness will be much more noticeable at low brightness values than in high brightness values. The API should specify whether the value is linear or logarithmic (linear makes sense to me - mapping this to a logarithmic scale is something that's maybe best left for application developers). |
For comparison, in github.com/aykevl/board, I've used There's also a Linux kernel API we could take a look at, but it's kinda terrible so perhaps we shouldn't. (If you ever wondered why Linux laptop brightness values are wonky, this is why: different vendors had different ideas and now nobody can agree on the meaning of "brightness"). |
I like the proposal to just remove backlight support from the displays all together, but I would like to add the proposal of than providing a separate backlight driver. How this is going to handle the multiple types of backlights (e.g. proportional via PWM or pin based levels) I'm not sure yet. I'm leaning towards having a Most systems I worked with don't abstract the logarithmic scale and let that for the app developer to handle, and I think that that is also what we should do here. That way the user keeps full control. Having worked with the linux kernel API, let's not go there. I am indeed the one you talked to about the DMA and PWM design. I gave my thoughts on the DMA part in the relevant issue. For the PWM I'm still debating on my stance. I'm leaning towards having an additional function to retrieve the PWM instance with a warning what that means with regards to changing multiple channels. Combining that with the way the |
I'm in favor of the |
To get a feel of what this could look like I've added a draft PR. The driver in the PR can be used as is or maybe passed to the display driver. The latter makes sense for displays that support multiple variants, e.g a display that supports PWM also supports on/off. But is not sensible for every combination. |
The backlight on most LCD screens (including the one on the Gopherbadge) is based on some form of LED control. The current support for this in the tinygo drivers is limited to turning the backlight on and off which is implemented for three screens in the repo.
When the pin connected to the backlight also has a PWM peripheral connected to it we could expand this functionality. My proposal would be to extend the API for these screens to allow for passing in a PWM interface similar to the
tone
driver and use that to set the display backlight to a certain value/percentage.From my experiments of enabling this on the users side we would need the following interface for the PWM:
Adding this to the existing API is a bit more difficult as I'm not sure what the guidelines are on breaking changes. One thing that could be done would be to have a separate initialisation call in the lines of
NewWithBacklightDimming
and make the functions that handle the backlight anoop
when the PWM interface has not been set. Optional names for these functions could beSetBacklight(percentage int8)
orDimBacklight(percentage int8)
.I'm not quite happy yet with how the above solution integrates with the current implementation. But I do think that this can add a valuable way to interact with these screens for users.
The text was updated successfully, but these errors were encountered: