[libcamera-devel] Fwd: Surface Go VCM type (was: Need to pass acpi_enforce_resources=lax on the Surface Go (version1))

[Daniel Scally]

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.


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 */
}


How do we know which one's the VCM when there's more than just two like
that? Andy: don't suppose you can shed any light there?

[1]
https://github.com/ZenfoneArea/android_kernel_asus_zenfone5/blob/master/linux/modules/camera/drivers/media/i2c/imx/dw9719.c
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-media-i2c-Add-driver-for-DW9719-VCM.patch
Type: text/x-patch
Size: 11350 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211029/9320c70e/attachment-0001.bin>