On Thu, 11 Nov 2021 at 16:50, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi, > > On 11/11/21 16:51, Dave Stevenson wrote: > > Hi Hans > > > > On Thu, 11 Nov 2021 at 15:23, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > >> > >> Hi, > >> > >> On 11/11/21 12:18, Daniel Scally wrote: > >> > >> <snip> > >> > >>>>> One problem I'm experiencing > >>>>> is that the focus position I set isn't maintained; it holds for a couple > >>>>> of seconds and then resets to the "normal" focus...this happens when the > >>>>> .close() callback for the driver is called, which happens right after > >>>>> the control value is applied. All the other VCM drivers in the kernel > >>>>> power down on .close() so I did the same> > >>>> Right, I believe that this is fine though, we expect people to use > >>>> libcamera with this and once libcamera gets autofocus support, then > >>>> I would expect libcamera to keep the fd open the entire time while > >>>> streaming. > >>> > >>> > >>> OK - as long as that's how it works then I agree that this is fine as is > >>> yes. > >> > >> So I've just picked up an old project of mine, called gtk-v4l which > >> is a nice simply v4l2 controls applet and patches it up to also > >> work on v4l-subdevs: > >> > >> https://github.com/jwrdegoede/gtk-v4l/ > >> > >> So now you can run: > >> > >> sudo gtk-v4l -d /dev/v4l-subdev8 > >> > >> And it will give you a slider to control the focus; and as > >> a bonus it keeps the v4l-subdev open, so no more runtime-pm > >> issue :) > > > > Do the lens and sensor share a regulator / enable GPIO? > > No, if they did then there would be no runtime-pm issue, > because then the VCM would not get turned off after > a v4l2-set command (for a quick test) since then the > streaming from the sensor would keep the sensor and > thus the regulator on. Registering with the regulator was more so that it restored the position on sensor power up, independent of whether the lens driver was opened or not. > > I was looking at the same issue for a Sony IMX135 module with AD5398 > > VCM driver [1]. > > In my case they do share an enable GPIO, so using regulator-gpio we > > can register via regulator_register_notifier for information on when > > the regulator is powered up. It can then also reset to the last > > position should the sensor subdev enable the regulator without the > > lens driver being opened at all. > > That sounds like it is relying on board-depedent behavior > (the enable GPIO and/or regulator being shared) which we don't > want in the VCM drivers as those are supposed to be board > agnostic. All platforms I've encountered so far have used the same GPIO to control both VCM and sensor, hence why I asked. The number of use cases where you want one without the other is incredibly low, and hardware guys generally don't like wasting GPIOs or having to route them around the PCB. It's interesting that your platform has separated them. > This really is something which should be fixed in userspace > where the userspace consumer of the sensor should also always > open the vcm v4l-subdev. Not all use cases involve libcamera, and what you're proposing is making life very difficult for the simple use cases. There may be GStreamer folk on board with libcamera, but I've heard no noises from FFmpeg about libcamera support. V4L2 is still the default API that users generally care about. Particularly with mono sensors the output is often directly usable without worrying about the complexities of ISPs, but you're effectively saying "jump through lots of hoops or you can't use a VCM with these sensors". If userspace has called VIDIOC_STREAMON doesn't that mean they want the whole entity (as configured) to be powered on and start streaming? Are you saying that the lens isn't part of that entity? In which case why does Media Controller include it (and eventually link it to the sensor) in the media entity? Would you advocate making backlight control in DRM a function that userspace is responsible for independently of the panel pipeline? There are significant similarities to this situation as the panel isn't usable without the backlight being powered, same as the sensor isn't usable without the VCM being powered. Sorry, but I just see isolating power control for the VCM from the sensor in this way to be a very odd design decision. It'd be interesting to hear other views. Dave
