Draft: hi846: add OTP interface and apply white balance gains during probe()
add OTP memory interface, read the module info and white balance data and calculate per-channel gains during probe().
Fixes: #294
Merge request reports
Activity
mentioned in merge request !358 (closed)
added camera label
the "SW user guide" by Hynix says Light source: DNP, Color temperature: 5000K±200K, Lux: 500Lux±50Lux amongh other things, but I guess you'd have to read it yourself for more details -.-
also, I have skipped the checksum verification for the OTP data reading for now but I'm very confident that this won't change anything. that said, I want to add it and clean up the code a bit more. (just a sidenote)
sorry, the vendor docuemnts the light source as: LED, 5100K±100K, Lux: 800±50 Lux. that should apply to our data.
Edited by Martin Kepplinger0x200
is 1x - we used to have it set at0x100
in the past, which was the reason why whites weren't white in our early images (see 365e9de8)And yes, ideally digital gain would stay at 1x. Megapixels already stores white balancing matrices in DNGs and those get used when dcraw produces JPEGs, but it gets them from the config file ignoring any unit-specific calibration (in fact at the moment it even uses a matrix for my Dogwood's rear cam :P)
Are you checking raw frames or what Megapixels produces? Raw data should be similar to the middle image here: https://source.puri.sm/Librem5/linux-next/-/issues/43#note_148338
Edited by Sebastian Krzyszkowiak
- Resolved by Martin Kepplinger
1501 struct i2c_client *c = v4l2_get_subdevdata(&hi846->sd); 1502 int ret; 1503 u16 i = 0; 1504 1505 if (!data || !len) 1506 return -EINVAL; 1507 1508 ret = hi846_write_reg(hi846, HI846_REG_FAST_STANDBY_MODE, 0x01); 1509 ret = hi846_write_reg(hi846, HI846_REG_MODE_SELECT, 0x00); 1510 msleep(20); 1511 ret = hi846_write_reg(hi846, 0x0f02, 0x00); /* pll disable */ 1512 ret = hi846_write_reg(hi846, 0x071a, 0x01); /* CP TRIM_H */ 1513 ret = hi846_write_reg(hi846, 0x071b, 0x09); /* IPGM TRIM_H */ 1514 ret = hi846_write_reg(hi846, 0x0d04, 0x00); /* Fsync (OTP busy) Output Enable */ 1515 ret = hi846_write_reg(hi846, 0x0d00, 0x07); /* Fsync (OTP busy) Output Drivaility */ 1516 ret = hi846_write_reg(hi846, 0x003e, 0x10); /* OTP r/w mode */ changed this line in version 5 of the diff
added 45082 commits
-
2af11c0a...01d72bf7 - 45080 commits from branch
Librem5:pureos/byzantium
- 63d212c6 - hi846: add OTP interface and apply white balance gains during probe()
- 88a4c991 - hi846: remove digital gain v4l control
-
2af11c0a...01d72bf7 - 45080 commits from branch
- Resolved by Martin Kepplinger
added 34 commits
-
88a4c991...95b9ae01 - 32 commits from branch
Librem5:pureos/byzantium
- 4547442c - hi846: add OTP interface and apply white balance gains during probe()
- 2d1fa9bb - hi846: remove digital gain v4l control
-
88a4c991...95b9ae01 - 32 commits from branch
added 1046 commits
-
2d1fa9bb...f7b8f817 - 1043 commits from branch
Librem5:pureos/byzantium
- fad3d0b2 - hi846: remove digital gain v4l control
- 84ce2a9e - hi846: add OTP interface and apply white balance gains during probe()
- 22c158d2 - hi8 debug output
Toggle commit list-
2d1fa9bb...f7b8f817 - 1043 commits from branch
reworked, cleaned up and added checksum validation.
This currently produces here:
[ 6.894956] hi846 2-0020: OTP awb gains: R:452 G:1039 B:676 [ 6.900666] hi846 2-0020: OTP awb golden gains: R:458 G:1023 B:666 [ 6.909188] hi846 2-0020: WB digital gain: R:526 G:520 B:512
But I might do something wrong still...
OK, with additional context this seems reasonable. Looks like we may even be able to use calibration data for Gb and Gr separately (that's what you print as
G
).It's not a set of gains that white balances the image, it's a set of gains that uses prerecorded white balance data in order to make the picture from each unit look as similar to each other as possible.
This is super useful ;)
To make sense of the captured colors and in turn to be able to properly white balance them, you need a color calibration matrix that's specific to particular sensor. A set of just three gain values is not enough, since you need to remap between different color spaces (there's a crosstalk happening between each color sensor and various cameras/color spaces/human eye cones react to different overlapping sections of the spectrum). I have created such (very rough) matrix for our rear camera already, but we eventually need to do it properly for both cameras ourselves and put it in Millipixels config.
However, each unit has small differences in sensitivity. In order to compensate for that, there's this OTP memory burned at production time. It works by taking a test photo of a white wall under specified lighting condition at factory and storing the average values for each color channel taken from a center area of the photo.
What's written into OTP memory is two sets of four values, one per each channel (R, Gr, Gb, B; where Gr are the green pixels next to red pixels, and Gb are green pixels next to blue pixels). The first set is a "golden" sample, the values that "should" have been reached with a hypothetical perfect specimen of our sensor. The second set is the actual value as seen by this particular unit.
Using this data, we can calculate a set of channel gains that will make our readings as close to "the perfect specimen" as possible, so we can only have a single color calibration matrix that will be good for every unit out there, provided that these gains are applied. Since the green channel is always the most sensitive, this is stored as ratios between red and blue channel values towards green (this way you save on one register while keeping the same information). Additionally, you also get a ratio between values from each green row (Gb/Gr). What the code in the datasheet is doing is basically recovering each channel reading from these ratios (by assuming that green is 100% saturated) and producing a set of ratios to translate between per-unit and golden ratios. Next, this is normalized to have the lowest value always at 1.0 in order to minimize dynamic range loss due to operating on discrete values coming from ADC.
Currently, you don't do anything with Gb/Gr ratio, just like the code in the datasheet, and treat both green channels as averaged together; but I believe that we actually could calculate the gains for each of the green channels separately as well, because we got that data too (there's averaged value you already calculate, a ratio between Gb and Gr, and combined set of ratios between channels that need to stay constant, so if I'm not mistaken you should be able to calculate distinct Gb and Gr gain values out of that too - but that can be done later, this is already good enough).
Edited by Sebastian Krzyszkowiak
1734 if (ret) 1735 return ret; 1736 1737 for (i = 0; i < 12; i++) 1738 sum += data[i]; 1739 1740 if (((sum % 255) + 1) == data[15]) { 1741 dev_dbg(&c->dev, "OTP read ok\n"); 1742 } else { 1743 dev_err(&c->dev, "OTP read failed\n"); 1744 return -EINVAL; 1745 } 1746 1747 R_unit = (data[0] << 8) | data[1]; 1748 B_unit = (data[2] << 8) | data[3]; 1749 Gr_Gb_unit = (data[4] << 8) | data[5]; 1741 dev_dbg(&c->dev, "OTP read ok\n"); 1742 } else { 1743 dev_err(&c->dev, "OTP read failed\n"); 1744 return -EINVAL; 1745 } 1746 1747 R_unit = (data[0] << 8) | data[1]; 1748 B_unit = (data[2] << 8) | data[3]; 1749 Gr_Gb_unit = (data[4] << 8) | data[5]; 1750 1751 dev_err(&c->dev, "OTP awb gains: R:%u G:%u B:%u\n", 1752 R_unit, Gr_Gb_unit, B_unit); 1753 1754 R_golden = (data[6] << 8) | data[7]; 1755 B_golden = (data[8] << 8) | data[9]; 1756 Gr_Gb_golden = (data[10] << 8) | data[11]; mentioned in issue #411
added 30794 commits
-
99983617...30a6e759 - 30792 commits from branch
Librem5:pureos/byzantium
- 97bd3d2b - hi846: remove digital gain v4l control
- 51e2c05e - hi846: add OTP interface and apply white balance gains during probe()
-
99983617...30a6e759 - 30792 commits from branch
added 34373 commits
-
51e2c05e...4aa18677 - 34371 commits from branch
Librem5:pureos/byzantium
- 33a2abeb - hi846: remove digital gain v4l control
- ee2797c0 - hi846: add OTP interface and apply white balance gains during probe()
-
51e2c05e...4aa18677 - 34371 commits from branch
added 15464 commits
-
ee2797c0...52719a38 - 15462 commits from branch
Librem5:pureos/byzantium
- 95438b4e - hi846: remove digital gain v4l control
- 0cefdc0b - hi846: add OTP interface and apply white balance gains during probe()
-
ee2797c0...52719a38 - 15462 commits from branch
added 19296 commits
-
0cefdc0b...263c3189 - 19294 commits from branch
Librem5:pureos/byzantium
- 1e288106 - hi846: remove digital gain v4l control
- 0f3a1c50 - hi846: add OTP interface and apply white balance gains during probe()
-
0cefdc0b...263c3189 - 19294 commits from branch
added 17826 commits
-
0f3a1c50...9ab8729f - 17824 commits from branch
Librem5:pureos/latest
- c2dd8773 - hi846: remove digital gain v4l control
- 9b176d94 - hi846: add OTP interface and apply white balance gains during probe()
-
0f3a1c50...9ab8729f - 17824 commits from branch
added 18797 commits
-
9b176d94...83bab2e2 - 18795 commits from branch
Librem5:pureos/latest
- 9e2b336e - hi846: remove digital gain v4l control
- 6d289c85 - hi846: add OTP interface and apply white balance gains during probe()
-
9b176d94...83bab2e2 - 18795 commits from branch