Re: [PATCH 1/2] drm/ssd130x: Scale ssd133x per-channel contrast by brightness on init

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The generic backlight update function corrupts the white balance on ssd133x panels by only updating contrast channel A.
--

--- Patch [1]: [PATCH 1/2] drm/ssd130x: Scale ssd133x per-channel contrast by brightness on init ---
commit 2b92cd2c08daa2f9c52218a1668057ae50f9c448
Author: Amit Barzilai <amit.barzilai22@gmail.com>

drm/ssd130x: Scale ssd133x per-channel contrast by brightness on init
    
This patch extracts the SSD133X A/B/C channel contrast calibration into
ssd133x_set_contrast() which scales each channel by the requested brightness.

> Extract them into ssd133x_set_contrast(), which scales each channel by a
> requested brightness via ssd130x_scale_contrast(), instead of writing the
> calibration unconditionally.  This makes the sequence readable, avoids
> repetition, and is a prerequisite for wiring up an ssd133x backlight
> controller that dims while preserving the white point.

[Severity: High]
This isn't a bug introduced by this patch, but does the generic backlight
update function corrupt the white balance on ssd133x panels by only updating
contrast channel A?

The commit message notes this is a prerequisite for a proper ssd133x backlight
controller, but currently ssd130x_probe() unconditionally registers the
generic backlight controller for all devices:

drivers/gpu/drm/solomon/ssd130x.c:ssd130x_probe() {
	...
	bl = devm_backlight_device_register(dev, dev_name(dev), dev, ssd130x,
					    &ssd130xfb_bl_ops, NULL);
	...
}

When the backlight is enabled or updated via sysfs, ssd130x_update_bl()
blindly writes the unscaled brightness to 0x81 (SSD13XX_CONTRAST). On ssd133x
panels, this maps to SSD133X_CONTRAST_A (Red):

drivers/gpu/drm/solomon/ssd130x.c:ssd130x_update_bl() {
	...
	ret = ssd130x_write_cmd(ssd130x, 1, SSD13XX_CONTRAST);
	if (ret < 0)
		return ret;

	ret = ssd130x_write_cmd(ssd130x, 1, ssd130x->contrast);
	...
}

This leaves channels B and C untouched at their initialization values,
destroying the white balance.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729053054.29374-1-amit.barzilai22@gmail.com?part=1