White point colour correction for Brick#760
Open
pvaibhav wants to merge 26 commits into
Open
Conversation
Add a Brick-only Display setting that toggles a system-wide white point correction on or off. The correction defaults to enabled and is applied during NextUI startup before normal UI/app use. The correction is implemented by displaycal.elf, which uploads a 256-entry packed RGB gamma LUT to the Allwinner display driver through /dev/disp using DISP_LCD_SET_GAMMA_TABLE and then enables gamma correction with DISP_LCD_GAMMA_CORRECTION_ENABLE. The LUT uses the Brick panel white point I measured for one of my Bricks, then applied to another Brick Hammer. I expect the variance between devices to be lesser than the overall distance of the panel from calibrated D65 white. The correction runs in hardware after upload, so it applies to NextUI, emulators, and third-party apps completely cost-free. Sleep/wake does not reset it. In this first iteration, Settings app only owns the public enabled=0/1 config and invokes displaycal.elf apply when the toggle changes. Currently no detailed calibration settings have been added.
Add white point red, green, and blue gain sliders in 0.01 steps to the Settings app (Display section). The correction remains enabled by default and reset restores the tuned defaults of red 1.00, green 0.92, and blue 0.58. Also saves the configured settings for r/g/b, not just enabled/disabled.
The white point defaults now live in a small shared header used by both displaycal.elf and Settings.cpp
When white point correction is turned off, displaycal now uploads an identity gamma table before issuing the disable ioctl. That prevents the previous correction table from hanging around in the display driver and interacting badly with color temperature or other panel controls. The explicit displaycal disable command follows the same path, while the identity command still leaves an identity table enabled for testing.
…writes Tighten displaycal.c's config grammar to the keys settings.cpp writes (enabled/red/green/blue), clamp parsed gains to the shared 0.00-2.00 range on both sides, and discard the tail of overlong config lines so it can't be parsed as its own line. Drop the now-unused screen, strength, and format keys along with their defaults. In the settings app, keep the config in an in-memory cache and flush (save + apply) once ~250ms after the last change instead of rewriting the file and forking displaycal.elf on every key-repeat tick. Log apply failures instead of discarding them, clamp gains as doubles before the int conversion to avoid UB on out-of-range values, and collapse the per-channel switches and copy-pasted menu items into a pointer-to-member accessor and a table loop.
The disp driver may reset the gamma LUT across suspend-to-RAM, so run displaycal apply at the start of the suspend script's wake path, gated the same way as the boot-time apply in launch.sh.
Persist Brick display calibration through the existing minuisettings.txt configuration path instead of maintaining a separate displaycal.cfg file and delayed dirty cache. The Settings UI now writes through CFG setters immediately and invokes displaycal.elf directly for live application. Simplify displaycal.elf to accept direct enable/disable commands with optional RGB gains, and update boot and resume hooks to reapply saved values through nextval.elf. This keeps boot, resume, and Settings using the same persisted source of truth.
There is currently no evidence that suspend/resume clears the display gamma LUT, and the suspend script does not reapply other display settings either. So for now, do not duplicate the boot-time displaycal parsing and application in the resume path. If we later find that the LUT really is lost across resume, we can add this back with that evidence instead of carrying the extra code preemptively.
Change the default display calibration gains from 1.0/1.0/0.6 to 1.0/0.92/0.58 in the shared display calibration defaults. Update the tg5040 launch fallback values to match the scaled minuisettings.txt representation so boot-time application uses the same defaults when settings are missing.
Add a 1.0.0 version string and --version handling to displaycal. Expand the usage text so it documents the supported commands, acceptable gain range, and suggested Brick RGB gain values, while keeping unknown arguments on the usage path.
Bump displaycal to 1.0.1 and add a short usage description explaining what it does.
Author
|
I removed the restoration of the LUT on resume in order to make a smaller PR. But it seems this is necessary as I’m noticing the first game start after resume from sleep resets the LUT. I’ll update the PR shortly. Need to revert that commit. |
Move display calibration LUT handling into a shared library used directly by libmsettings, and persist the white point settings in the tg5040 msettings state alongside the existing display controls. Reapply calibration through the existing SetMute display replay path so the LUT is restored whenever brightness, color temperature, contrast, saturation, and exposure are replayed after resume-sensitive display updates. Remove the standalone displaycal CLI and the launch-script/nextval plumbing because runtime no longer needs to parse separate configuration or fork a helper. Settings now read and write the new libmsettings getters/setters directly, with Brick-only UI exposure and raw hardware application still guarded by DEVICE=brick.
Keep the shared display calibration helper focused on scaled integer gain values and hide the floating-point LUT representation inside the implementation. Move Brick-specific defaults back into tg5040 msettings so the common header does not encode device policy or become part of libmsettings' public include surface. Guard the Settings UI integration behind a tg5040 display calibration build flag while retaining the runtime device gate, and remove Brick-specific wording from user-visible copy. Restore whitespace-only config hunks so those files do not carry PR noise against main.
Remove the stale top-level packaging copy for displaycal.elf. Display calibration is now linked into libmsettings directly, so clean tg5040 package builds no longer produce a standalone displaycal executable to copy.
Replace the custom decimal-formatted gain option lists with the built-in 0-200 integer range menus for white point correction. This removes the unused formatting helper API and updates menu descriptions and comments to explain that 100 is the neutral gain value.
Make the white point correction controls available on every settings build while keeping the feature disabled with neutral 100/100/100 gains by default outside Brick. Brick continues to receive the measured 100/92/58 correction and enabled-by-default behavior. Persist display calibration values in the tg5050 and desktop msettings implementations. Apply calibration on tg5040 and tg5050 whenever the setting changes and when display state is reapplied through SetMute or boot initialization; desktop keeps the API as a logged no-op for local use.
Add a small applyDisplayCalSettings helper to the platform msettings implementations so display calibration setters, boot reapply, and mute reapply all use the same raw-apply path. This removes the repeated SetRawDisplayCal(GetDisplayCal...) orchestration while preserving the existing save behavior and keeping desktop as a log-only no-op backend.
Capture the return code from display calibration enable and disable calls in the hardware-backed msettings implementations. This keeps the existing void SetRawDisplayCal API but surfaces failed raw applies at the msettings layer so settings changes are easier to diagnose when the device LUT operation fails.
Make the white point correction menu getter return a bool to match its {false, true} value list. This fixes the MenuItem type assertion on Settings startup while preserving the stored integer msettings representation.
Move the display calibration gain clamp into the shared header so desktop can reuse the same helper without linking the hardware LUT backend. Update the desktop msettings build include path and replace the local 0-200 clamp checks in its RGB gain setters.
Add an explicit hasDisplayCal capability helper and use it to guard the white point correction toggle and RGB gain items, matching the surrounding display menu capability checks. Document that the feature is exposed everywhere, that desktop raw apply only logs, and that menu reset defaults come from the display calibration default helpers.
Move neutral and Brick-specific display calibration defaults into the shared displaycal header so settings reset and tg5040 migration/init paths use the same values. Remove the duplicated SETTINGS_DEFAULT_DISPLAYCAL_* definitions from platform msettings headers and update libmsettings initializers to read the shared constants.
Use the DeviceInfo enum scope when checking whether the settings UI is running on Brick. This fixes the display calibration defaults refactor build failure where Brick was referenced outside the class scope.
Author
|
I have reworked the entire patch as follows:
|
Skip raw display calibration writes when calibration is already disabled so boot and repeated reset paths do not issue unnecessary LUT or disable ioctls. Keep an explicit disable write for live on-to-off transitions, and mirror the behavior in desktop logging for parity.
Author
|
The last change now also sets white point correction on boot up only when it's actually enabled. This avoids touching the gamma ioctls completely on devices unless it's explicitly enabled (by the user or by the system). If it's disabled, on boot up we don't need to touch the gamma tables. An on-to-off transition while the device is already booted up will still write to the gamma LUT before disabling, as it's necessary to clear the corrections. Otherwise, colour temperature values other than 20 (the mid point) will continue to have the LUT applied. This is safe because someone must have enabled it first (either the user or the system) so any damage has probably already occurred. 😃 This probably needs more testing than I did, but seeing as nothing else overwrites the gamma LUT, should be safe. |
Hide white point correction for tg5050 because the platform does not expose /dev/disp. Make the tg5050 settings backend force display calibration off, keep raw display calibration as an unsupported no-op, and stop linking displaycal.c for that target while preserving tg5040 support.
Move the red, green, and blue display calibration gain controls to the bottom of the Display settings menu before reset. Simplify their descriptions by removing the neutral-value wording.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Problem
The TimUI Brick has LCD panels that have too much blue tint. The measured white point was (0.279, 0.278) confirming this. The system-provided colour temperature adjustment slider only moves the white point along the blue/red axis, but the green is what needs correction.
The solution
I investigated various methods for correcting this, including a shader that works okay in gameplay (albeit with some performance penalty). It became clear that a system-wide method is needed. With the help of Codex I found out that there exists display gamma ioctls in /dev/disp that seem to do exactly that: update a look-up table that is applied system-wide at the hardware level, with zero performance penalty.
The implementation
With this finding, a small utility
displaycal.cis implemented which can calculate a LUT and upload it to the display based on red/green/blue gains. These settings are reapplied on boot via MinUI launch system.Next, the Settings app and config system was updated, to allow the user to configure their preferences (Settings > Display section). A separate pak could perform all these steps using hooks, but in my opinion this fragments display settings into two places for not much benefit.
Note that white point correction does not interfere with any existing controls like the colour temperature. Those get applied on top, so this change also makes the colour temperature slider more accurate.
Default values
Once merged, white point correction will be enabled by default for all users on TrimUI Brick (only). All other devices do not get this feature yet - unless it's verified that it works on other devices. It should work on all Allwinner devices but at least the default values will be different.
Using data from two Bricks (one a regular and another a Brick Hammer) I've verified that the defaults I've chosen (r=1.0, g=0.92, b=0.58) move the white point back towards D65 (0.3127, 0.3290). The colorimeter used was an inexpensive i1 Display Pro however I will refine this with a Konica Minolta sensor. In any case, the per-unit variation is expected to be significantly less than the overall correction of reducing around 40% of blue.
Future work
Two main areas --