Re: Fwd: Surface Go VCM type (was: Need to pass acpi_enforce_resources=lax on the Surface Go (version1))

[Daniel Scally <djrscally@gmail.com>]

Hi Hans

On 04/11/2021 14:49, Hans de Goede wrote:
> Hi Daniel,
>
> On 11/2/21 00:43, Daniel Scally wrote:
>> Hi Hans
>>
>> On 01/11/2021 16:02, Hans de Goede wrote:
> <snip>
>
>>>> 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.
>> Oh really, I'll test that too; thanks for the patches. There's a couple
>> of tweaks to the driver anyway, so hopefully be able to get it ironed out.
> Ok, I've figured this out now, with the attached patch (which also
> explains what is going on) as well as an updated tps68470_board_data.c
> with updated regulator_init_data for the VCM (also attached), the driver
> can now successfully talk to the VCM in probe() while we are NOT
> streaming from the ov8865.
>
> Daniel, please feel free to squash this into your original dw9719 patch.
>

Nice thanks - I'll do that.

>
>>> 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 ?
>>
>> I think on top of your work in the cio2-bridge for patch 3 you can do this:
>>
>>
>> 1. Create another software node against the cio2_sensor struct, with the
>> name coming from the vcm_types array
>>
>> 2. Assign that software node to board_info.swnode in
>> cio2_bridge_instantiate_vcm_i2c_client()
>>
>> 3. Add another entry to dev_properties for the sensor, that is named
>> "lens-focus" and contains a reference to the software_node created in #2
>> just like the references to the sensor/cio2 nodes.
>>
>>
>> This way when the sensor driver calls
>> v4l2_async_register_subdev_sensor() it should create a notifier that
>> looks for that VCM client to bind. I think then rather than putting
>> anything in the .bound() / .complete() callbacks, we should modify core
>> to do _something_ when async matching some subdevs. The something would
>> depend on the kind of devices that match, for example with the sensor
>> driver and the ipu3-cio2 driver, there's an entity whos function is
>> MEDIA_ENT_F_VID_IF_BRIDGE matching to an entity whos function is
>> MEDIA_ENT_F_CAM_SENSOR, and it seems to me that every scenario like that
>> is going to result in media pad links being created. Similarly for our
>> sensor that's a device with entity function MEDIA_ENT_F_LENS matching to
>> MEDIA_ENT_F_CAM_SENSOR, and I think that in those cases we can create
>> either an interface link or a new kind of link (maybe
>> "MEDIA_LNK_FL_ANCILLARY_LINK" or something...) between the two to show
>> that they form a single logical unit, which we can then report to libcamera.
>>
>>
>> Hope that makes sense...
> Maybe? I have not looked into this closely yet. I'll continue working on
> this coming Tuesday.
>
> If you feel like tinkering I would not mind if you beat me to it this
> weekend :)   OTOH please enjoy your weekend doing whatever, I can continue
> working on this during office-hours next week.


I'll probably have some time to look at it over the next few days; I'll
let you know how I get on.

>
> Regards,
>
> Hans