Skip to content
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

[2.3.2.r1.4] msm: camera: actuator: Fix range for BU64747 actuator #1921

Merged

Conversation

kholk
Copy link
Contributor

@kholk kholk commented Feb 22, 2019

The actuator has a sensible range of 250-800, where 800 is
for near subjects and 250 is "infinity".

Address the userspace framework issues with the actuator range
by normalizing the incoming position request data in the
supported range.

Tested on SoMC Tama Akatsuki RoW.

@kholk
Copy link
Contributor Author

kholk commented Feb 22, 2019

@sraase Due to the issues with the actuator range, this commit appeared to be totally necessary.

Currently, the framework sends a range of 414-699.
The actuator accepts at least a range of 1-800 but, from my testing, having it that much wide doesn't make sense.
The range that actually makes sense is around 250 for infinity, to 800 for macro.

I am not sure (but I sort of hope) that this can be addressed in the userspace framework in a timely manner, so I'd like to have your opinion on this, as well.

Thanks ;)

@sraase
Copy link
Contributor

sraase commented Feb 26, 2019

On Tama, userspace uses the AF calibration data from EEPROM, i.e. known DAC values for macro and infinity. The framework maps those onto a 400 step step range, which the algorithms operate on. The v6 image introduced AF tuning values as well.

When you set persist.vendor.camera.logVerboseMask to 0x2, restart the camera server and start the camera app, you should see the mapping in the log. On my Akari test phone, I see values between ~310 and ~780, and you should see similar values based on your testing. Could you please verify this, and do you see any difference when running with the "default" tuning (remove all other tuning files)?

If needed, I can measure the AF values before the next release, but that requires ruling out a systematic error on my side first. I don't think patching the kernel is a good idea, but it may suffice as a stop-gap measure.

Also, thanks for pinging me on the comment; I won't notice otherwise. :-)

@kholk
Copy link
Contributor Author

kholk commented Feb 27, 2019

@sraase
You're welcome!

So, I tried to set the verbose mask and yeah now the output is really verbose....
...though, I cannot see/locate the actuator values in logcat, even if I try to set the logVerboseMask to UINT32_MAX (4294967295 -- 0xFFFFFFFF).
About your request to remove the calibrations... nnnoupe, unless I got it wrong, removing com.qti.tuned.beagle.bin doesn't change the actuator ranges that the userspace framework spits to the kernel driver. Still the same range.

By the way, I had this hack way before v6 was released (so the calibrations weren't there at all). I have pushed this thing after seeing the latest ODM drop because I was thinking that this was something that you were solving with the actual calibration, which it wasn't the case.
I was also totally ashamed to push it, because... I'm sure that you get my feelings about this solution. I sort of hate it.

One last thing!
I want to make sure that you understand what I'm talking about, when I say things about "the actuator range" that I've retrieved: I didn't pull the values from the userspace but from the kernel, by forcing the set dac message to CAM_ERR (to avoid seeing an infinite amount of spam....).

Example:
[ 199.165724] CAM_ERR: CAM-ACTUATOR: bu64747_set_dac: 180 bu64747: set new dac value 560. [ 199.239858] CAM_ERR: CAM-ACTUATOR: bu64747_set_dac: 180 bu64747: set new dac value 567. [ 199.314287] CAM_ERR: CAM-ACTUATOR: bu64747_set_dac: 180 bu64747: set new dac value 571. [ 199.396675] CAM_ERR: CAM-ACTUATOR: bu64747_set_dac: 180 bu64747: set new dac value 578.

In any case, I'm forcepushing the commit with a truly needed fix... I've just noticed that I am forcing the lens to never go to the zero position (which happens to be requested when you close the camera app)... so it never got parked correctly! Oops! :P

The actuator has a sensible range of 250-800, where 800 is
for near subjects and 250 is "infinity".

Address the userspace framework issues with the actuator range
by normalizing the incoming position request data in the
supported range.
@kholk
Copy link
Contributor Author

kholk commented Feb 27, 2019

To anyone using this PR:
The commit has been forcepushed. Now the driver allows to park the lens in the right position, as requested by the framework.

@sraase
Copy link
Contributor

sraase commented Feb 27, 2019

@kholk The userspace mapping is not shown until you open the camera. You should see lines like this:
CamX : [ VERB][SENSOR ] camxactuatordata.cpp:176 InitializeStepTable() Table[399] 348
Apparently, the verbosity is high enough to overrun logcat's abilities even with a larger buffer, but at least some of the lines should make it through. I don't know if AF tuning data changes this table, so please check with and without tuning.

As far as I can tell, there are at least four layers to consider:

  • hardware driver (kernel & userspace, using DAC values)
  • camera framework and 3A (measuring in steps 0..399)
  • android framework (measuring in meters)

I only know very little about the latter two layers.

On my device, the mapping table goes from 348 to 841. When doing a sweep from an Android app, the DAC values are 484-719-476 (step values 289-99-295). So even though the camera framework knows the actuator range, it will only use a small part of it. So the problems seems not to be in the driver, at least.

@kholk
Copy link
Contributor Author

kholk commented Mar 8, 2019

@sraase
Sorry for the big delay, I've been a lil busy in these days.
Anyway, I have been able to retrieve the data that you asked!

With com.qti.tuned.beagle.bin:
InitializeStepTable() Table[399] 338 ..... InitializeStepTable() Table[0] 812

Without com.qti.tuned.beagle.bin:
InitializeStepTable() Table[399] 338 ..... InitializeStepTable() Table[0] 812

So it looks the... exact.. same :)
And yeah, I agree that the problem is not in the kernel driver at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants