[libcamera-devel] Fwd: Surface Go VCM type (was: Need to pass acpi_enforce_resources=lax on the Surface Go (version1))
Hans de Goede
hdegoede at redhat.com
Mon Nov 1 17:02:58 CET 2021
On 10/29/21 13:50, Daniel Scally wrote:
> Hi all
>
>
> +CC linux-media and libcamera-devel, as it's probably a good time to
> broaden this out. Also Andy because I'm hoping you can help :) The
> background of the discussion is about how we identify and enumerate
> (correctly, I.E. with a type matching the vcm driver's i2c_device_id,
> and there are a few different vcm's in scope which seem encoded in the
> SSDB buffer) which VCM module is linked to a sensor in Intel's IPU3
> centric ACPI tables. The I2C address for the device is just a second
> I2cSerialBusV2 against the sensor's acpi device rather than a separate
> one, which is no awkward. We also need to get firmware created for the
> VCM such that the sensor will link to it via the lens-focus property.
>
> On 28/10/2021 09:57, Hans de Goede wrote:
>> Hi,
>>
>> On 10/28/21 10:49, Laurent Pinchart wrote:
>>> Hi Hans,
>>>
>>> On Thu, Oct 28, 2021 at 09:51:08AM +0200, Hans de Goede wrote:
>>>> On 10/28/21 09:10, Daniel Scally wrote:
>>>>> On 27/10/2021 15:16, Hans de Goede wrote:
>>>>>> On 10/27/21 12:07, Daniel Scally wrote:
>>>>>>> On 26/10/2021 11:14, Hans de Goede wrote:
>>>>>>>>>> So yesterday I already sorta guessed it would be the DW9714 because of
>>>>>>>>>> the 0x0c address and I tried:
>>>>>>>>>>
>>>>>>>>>> i2ctransfer -y 2 w2 at 0x0c 0x00 0x00
>>>>>>>>>>
>>>>>>>>>> And the transfer fails, while according to the driver that is a valid
>>>>>>>>>> value. So maybe we are missing a regulator enable? Or its not a DW9714.
>>>>>>>>>>
>>>>>>>>>> Also "i2cdetect -y -r 2" does not see anything at address 0x0c (but some of
>>>>>>>>>> these VCMs seem to be write only...) it does OTOH see an unknown device at
>>>>>>>>>> address 0x21.
>>>>>>>>> Well, when debugging the necessary TPS68470 settings I used a poor man's
>>>>>>>>> i2ctransfer on Windows whilst the camera was running to read the values
>>>>>>>>> that were set for both the PMIC and the camera sensor. Using the same
>>>>>>>>> program I can connect to and read values from a device at 0x0c,
>>>>>>> Just as further testing I dumped the contents of the device at 0x0c,
>>>>>>> which comes back as
>>>>>>>
>>>>>>> f1 1 2 1 61 0 40 60
>>>>>>>
>>>>>>> Byte 0 is given in the driver you linked as the ID field and expected to
>>>>>>> be f1. The driver controls focus by writing to the 3rd and 4th byte
>>>>>>> (with the 4th being the LSB); the only value that seemed to fluctuate
>>>>>>> when running windows and moving my hand in front of the sensor was byte
>>>>>>> 4 and testing it out I wrote values into that byte and the focus
>>>>>>> changes. So the device at 0x0c is definitely the vcm and it sure looks
>>>>>>> like it's the DW9719
>>>>>>>
>>>>>>> The device at 0x21 is only available on Windows when the camera is
>>>>>>> running, I thought it was quite likely that one of the "spare"
>>>>>>> regulators from the TPS68470. One line is called VCM, and sure enough
>>>>>>> it's enabled whilst the world-facing camera is running. I switched to
>>>>>>> linux and started streaming the back camera, then enabled that voltage
>>>>>>> regulator via i2ctransfer:
>>>>>>>
>>>>>>> sudo i2ctransfer 2 w2 at 0x4d 0x3c 0x6d
>>>>>>>
>>>>>>> sudo i2ctransfer 2 w2 at 0x4d 0x44 0x01
>>>>>>>
>>>>>>> And now i2cdetect shows the device at 0x0c on bus 2 - so we need more
>>>>>>> jiggery pokey to map that VCM regulator to this new device (once we've
>>>>>>> gotten it enumerated...) and the driver needs to have a tweak to call
>>>>>>> regulator get and do a power on at some point.
>>>>>> Awesome, great job on figuring this out!
>>>>>>
>>>>>> As you know I can spend $dayjob time on this, so I'll take on the job
>>>>>> of creating the i2c-client and hooking up the regulator in some
>>>>>> upstreamable manner.
>>>>> Okedokey cool. I'd probably start at the cio2-bridge, if only because we
>>>>> already have the adev there and the SSDB buffer loaded, so should be
>>>>> easy enough to add an enum for the vcm_type and a call to
>>>>> i2c_acpi_new_device()...bit of a weird place for that though I guess.
>>>> Ah, I was actually thinking about doing this int he int3472 code for
>>>> a number of reasons:
>>>>
>>>> 1. We already have the regulator_init_data there and we will need to
>>>> expand it for this.
>>>>
>>>> 2. It is sorta the central place where we deal with all this glue-stuff
>>> I'm not too sure about that. The INT3472 model the "Intel camera PMIC"
>>> (I don't remember the exact wording, but that's more or less how the
>>> device is described in Windows, and it matches the intent we see in the
>>> DSDT).
>> I agree that the INT3472 models the PMIC, or whatever discrete bits
>> which offer similar functionality.
>>
>>> Given that we already have cio2-bridge, and that it hooks up the
>>> sensor to the CIO2, it seems to me that it would be a better central
>>> place.
>> Ok, I was sorta expecting you to want to keep glue code like this
>> out of drivers/media. But I guess that only applies to putting ACPI
>> specific stuff in sensor drivers; and since the cio2-bridge code is
>> already x86/ACPI specific you are fine with adding ACPI code there?
>>
>> I'm fine with putting the VCM i2c-client instantiation in the
>> cio2-bridge code, that may also make it easier to tie the 2 together
>> at the media-controller level.
>
>
> Having looked at this yesterday evening I'm more and more convinced it's
> necessary. I hacked it into the ov8865 driver in the interim (just by
> calling i2c_acpi_new_device() in probe) and then worked on that dw9719
> code you found [1] to turn it into an i2c driver (attached, though still
> needs a bit of work), which will successfully bind to the i2c client
> enumerated by that i2c_acpi_new_device() call. From there though it
> needs a way for the v4l2 subdev to be matched to the sensor's subdev.
> This can happen automatically by way of the lens-focus firmware property
> against the sensor - we currently build those in the cio2-bridge, so
> adding another software node for the VCM and creating a lens-focus
> property for the sensor's software_node with a pointer to the VCM's node
> seems like the best way to do that.
So besides prepping a v5 of my previous series, with update regulator
init-data for the VCM I've also been looking into this, attached are
the results.
Some notes from initial testing:
1. The driver you attached will only successful probe if I insmod
it while streaming video from the sensor. So I think we need another
regulator or the clk for just the VCM too, I will investigate this
later this week.
2. I need some help with all the fwnode link stuff (I'm not very familiar
with this). There seems to be a chicken and egg problem here though,
because the v4l2subdev for the VCM does not register because of async stuff
and if we add it to the "graph" then my idea to enumerate the VCMs
from the SSDB on the complete() callback won't work. But we can do this
on a per sensor basis instead from the cio2_notifier_bound() callback
instead I guess ?
Can someone give me some hints how the fwnode link code should look/work
and how I can get the async registering of the subdev for the VCM to
complete ?
Regards,
Hans
>
>
> To throw a spanner in the works though; I noticed this delightful _CRS
> for the OV9734 sensor of a Surface Laptop 1 earlier:
>
>
> Method (_CRS, 0, Serialized) // _CRS: Current Resource Settings
> {
> Name (SBUF, ResourceTemplate ()
> {
> I2cSerialBusV2 (0x0036, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C2",
> 0x00, ResourceConsumer, , Exclusive,
> )
> I2cSerialBusV2 (0x0050, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C2",
> 0x00, ResourceConsumer, , Exclusive,
> )
> I2cSerialBusV2 (0x0051, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C2",
> 0x00, ResourceConsumer, , Exclusive,
> )
> I2cSerialBusV2 (0x0052, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C2",
> 0x00, ResourceConsumer, , Exclusive,
> )
> I2cSerialBusV2 (0x0053, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C2",
> 0x00, ResourceConsumer, , Exclusive,
> )
> })
> Return (SBUF) /* \_SB_.PCI0.I2C2.CAMF._CRS.SBUF */
> }
Hmm, we do have i2c_acpi_client_count(adev), so it is easy to use
that and just always use the last resource for the VCM. But that assumes
that is what is going on here and I have no idea.
Regards,
Hans
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-i2c-acpi-Change-first-param-of-i2c_acpi_new_device-t.patch
Type: text/x-patch
Size: 7958 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211101/1cdf5afe/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-media-ipu3-cio2-Store-cio2_bridge-pointer-in-struct-.patch
Type: text/x-patch
Size: 3648 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211101/1cdf5afe/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-media-ipu3-cio2-Add-support-for-instantiating-i2c-cl.patch
Type: text/x-patch
Size: 5524 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211101/1cdf5afe/attachment-0005.bin>
More information about the libcamera-devel
mailing list