Hi Volodymyr, On 20/07/2022 16:44, Volodymyr Kharuk wrote: > Hi Benjamin, > > On Tue, Jul 19, 2022 at 05:20:40PM +0200, Benjamin Mugnier wrote: >> Hi Volodymyr, >> >> >> On 15/07/2022 17:03, Volodymyr Kharuk wrote: >>> On Fri, Jul 15, 2022 at 12:36:18PM +0300, Laurent Pinchart wrote: >>>> Hello, >>>> >>>> CC'ing Benjamin Mugnier who I recall expressed an interest in ToF >>>> sensors (if I recall incorrectly, my apologies). >> >> I am indeed very interested. Thank you :) >> >>>> >>>> On Fri, Jul 15, 2022 at 11:57:20AM +0300, Volodymyr Kharuk wrote: >>>>> On Thu, Jul 14, 2022 at 01:31:35PM +0300, Laurent Pinchart wrote: >>>>>> On Thu, Jul 14, 2022 at 11:34:46AM +0300, Volodymyr Kharuk wrote: >>>>>>> Define user controls for mlx7502x driver and update MAINTAINERS >>>>>>> >>>>>>> Signed-off-by: Volodymyr Kharuk <vkh@xxxxxxxxxxx> >>>>>>> --- >>>>>>> MAINTAINERS | 7 +++++++ >>>>>>> include/uapi/linux/mlx7502x.h | 31 +++++++++++++++++++++++++++++++ >>>>>>> 2 files changed, 38 insertions(+) >>>>>>> create mode 100644 include/uapi/linux/mlx7502x.h >>>>>>> >>>>>>> diff --git a/MAINTAINERS b/MAINTAINERS >>>>>>> index ef3ec334fae9..1a68d888ee14 100644 >>>>>>> --- a/MAINTAINERS >>>>>>> +++ b/MAINTAINERS >>>>>>> @@ -12673,6 +12673,13 @@ S: Supported >>>>>>> W: http://www.melexis.com >>>>>>> F: drivers/iio/temperature/mlx90632.c >>>>>>> >>>>>>> +MELEXIS MLX7502X DRIVER >>>>>>> +M: Volodymyr Kharuk <vkh@xxxxxxxxxxx> >>>>>>> +L: linux-media@xxxxxxxxxxxxxxx >>>>>>> +S: Supported >>>>>>> +W: http://www.melexis.com >>>>>>> +F: include/uapi/linux/mlx7502x.h >>>>>>> + >>>>>>> MELFAS MIP4 TOUCHSCREEN DRIVER >>>>>>> M: Sangwon Jee <jeesw@xxxxxxxxxx> >>>>>>> S: Supported >>>>>>> diff --git a/include/uapi/linux/mlx7502x.h b/include/uapi/linux/mlx7502x.h >>>>>>> new file mode 100644 >>>>>>> index 000000000000..44386f3d6f5a >>>>>>> --- /dev/null >>>>>>> +++ b/include/uapi/linux/mlx7502x.h >>>>>>> @@ -0,0 +1,31 @@ >>>>>>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ >>>>>>> +/* >>>>>>> + * Melexis 7502x ToF cameras driver. >>>>>>> + * >>>>>>> + * Copyright (C) 2021 Melexis N.V. >>>>>>> + * >>>>>>> + */ >>>>>>> + >>>>>>> +#ifndef __UAPI_MLX7502X_H_ >>>>>>> +#define __UAPI_MLX7502X_H_ >>>>>>> + >>>>>>> +#include <linux/v4l2-controls.h> >>>>>>> + >>>>>> >>>>>> These controls should be documented, in >>>>>> Documentation/userspace-api/media/drivers/. >>>>> >>>>> Ok, will do in v3 >>>>> >>>>>>> +/* number of phases per frame: 1..8 */ >>>>>>> +#define V4L2_CID_MLX7502X_PHASE_NUMBER (V4L2_CID_USER_MLX7502X_BASE + 0) >>>>>>> +/* shift of each phase in frame, this is an array of 8 elements, each 16bits */ >>>>>>> +#define V4L2_CID_MLX7502X_PHASE_SEQ (V4L2_CID_USER_MLX7502X_BASE + 1) >>>>>>> +/* frequency modulation in MHz */ >>>>>>> +#define V4L2_CID_MLX7502X_FMOD (V4L2_CID_USER_MLX7502X_BASE + 2) >>>>>>> +/* time integration of each phase in us */ >>>>>>> +#define V4L2_CID_MLX7502X_TINT (V4L2_CID_USER_MLX7502X_BASE + 3) >>>>>> >>>>>> Are these control very device-specific, or are they concept that apply >>>>>> in general to ToF sensors ? Same for V4L2_CID_MLX7502X_OUTPUT_MODE. >>>>> >>>>> These controls(except V4L2_CID_MLX7502X_OUTPUT_MODE) are general for ToF >>>>> sensors. Do you think we should standardize them? >>>> >>>> I would really really like to see control standardization for ToF >>>> sensors, yes :-) >>> Sounds great :) >> >> Thanks a lot for your efforts in standardizing these controls. This is pretty close to what I expected :) >> >> Sensors may require multiple fmod from the user, and may not be able to deduce them from a single one. >> Subframes may be acquired for each fmod (composed themselves of acquisitions for each phase), and then generate a frame from these. >> Here is a quick drawing example with 2 fmod and 2 phases. Hope this makes sense. >> >> |-------------------------------------------------------------------------> time >> |FMOD1 PHASE1|FMOD1 PHASE2|FMOD2 PHASE1|FMOD2 PHASE2|FMOD1 PHASE1|... >> | SUBFRAME1 | SUBFRAME2 | >> | FRAME1 | > Just to be clear: one camera trigger generates one frame. > The number of subframes can be configured via register. Yes exactly. It's mostly vocabulary differences afaik, but we are on the same > One phase generates one image. Do we have the same understanding? Yes, one frame on the MIPI is one phase. The reconstitution is done later on. But some sensors tend to put their "start of frame" and "end of frame" markers for the concatenation of subframes (the frame on my drawing), and not for each phase. I'm not really familiar with this yet. This is used example if you want to plug your sensor in a software stack that can't handle more than a limited number of fps. You trick it into getting one big frame and then slice after. This is afaik the same mechanism as used for high fps cameras. I don't really know how to handle this kind of usage yet. >> >> This allows greater ranges. >> I suggest changing V4L2_CID_MLX7502X_FMOD to an array, if it suits you. >> I'm curious how are you doing this? Are you using only one fmod or do you compute some others from the first one? Either in the driver or the sensor. > The mlx7502x supports only one fmod per frame. Instead it is possible to use > a few phase shifts(V4L2_CID_MLX7502X_PHASE_SEQ) per one frame. Using > cross-correlation it is possible to calculate the distance from received images. Alright. In my opinion if we standardize this control we could make it an array from the start to prepare for other sensors using multiple fmod. >> >>>> >>>> Do you know of any public litterature that explains the operating >>>> principles of ToF sensors ? I don't expect most of the V4L2 developers >>>> to be familiar with the concept, so something that could bring us up to >>>> speed on ToF would be useful for the discussion. >>> >>> Here what I have: >>> 1. ToF Basics from Melexis >>> https://media.melexis.com/-/media/files/documents/application-notes/time-of-flight-basics-application-note-melexis.pdf >>> 2. ToF Basics from TI >>> https://www.ti.com/lit/wp/sloa190b/sloa190b.pdf?ts=1657842732275&ref_url=https%253A%252F%252Fwww.google.com%252F >>> 2. ToF systems from TI >>> https://www.ti.com/lit/ug/sbau219d/sbau219d.pdf >>> 4. This more related to ToF algorithms >>> https://hal.inria.fr/hal-00725654/document >>> >>> I hope it helps. >>>> >>>>> Note that the control V4L2_CID_MLX7502X_TINT is similar to >>>>> V4L2_CID_EXPOSURE, but the way it is done in ToF is different. They don't >>>>> have a shutter. So I gave a separate control name. Is it ok? >>>> >>>> Yes, I think that's fine. >>>> >> >> Having only one integration time control is problematic for HDR sensors as they require both a short and long integration time setting. >> I have the same issue for the vgxy61 camera with V4L2_CID_EXPOSURE and ended up defining 2 custom controls for both short and long exposure, but I understand this is not ideal. Maybe Laurent have an idea on this? > It is possible to have integration time per phase. For now I just copy tint into all phase registers. Yes I see little to no use cases for variable integration time per phase. So your control is fine. For HDR it's kind of special, as every frame is acquired is in fact 2 frames: one in HDR and one in SDR, and they require 2 different integration time. As I said the problem already exists for camera sensors, so this is fine as is for now :) >> >>>>>>> +/* mode could sw(sending i2c packet), hw(pin triggering), and continuous(self triggering) */ >>>>>>> +#define V4L2_CID_MLX7502X_TRIGGER_MODE (V4L2_CID_USER_MLX7502X_BASE + 4) >>>>>>> +/* in case sw or hw trigger mode is used */ >>>>>>> +#define V4L2_CID_MLX7502X_TRIGGER (V4L2_CID_USER_MLX7502X_BASE + 5) >>>>>> >>>>>> Trigger control is likely something we need to standardize at the V4L2 >>>>>> level. >>>>> >>>>> Ok, then I'll remove these controls for now and I will back with this as >>>>> a separate patch. >>>>> >>>>>>> +/* this is related to the taps in ToF cameras, usually A minus B is the best option */ >>>>>>> +#define V4L2_CID_MLX7502X_OUTPUT_MODE (V4L2_CID_USER_MLX7502X_BASE + 6) >>>>>>> +/* ToF camers has its own temperature sensor, which can be read out only during streaming */ >>>>>>> +#define V4L2_CID_MLX7502X_TEMPERATURE (V4L2_CID_USER_MLX7502X_BASE + 7) >>>>>> >>>>>> This should probably use the proposed temperature control from >>>>>> https://lore.kernel.org/linux-media/20220415111845.27130-3-benjamin.mugnier@xxxxxxxxxxx/ >>>>> >>>>> Ok, then I'll remove these controls for now. >>>>> >> >> We discussed the standardization of the temperature control with linux-hwmon subsystem team [1]. >> As this happened to be a trickier problem than I thought, I decided to remove the temperature control I initially proposed. You can find the v3 of the vgxy61 without the temperature control [2]. >> >> So no temperature control for now. I plan on giving it another go after the vgxy61 is accepted, but for now the simpler the better. >> Of course feel free to do it, I'll gratefully rebase on your work ;) >> >> [1] https://lore.kernel.org/linux-media/d4c868d5ef05f338bdc2237d9b9304077d268c8b.camel@xxxxxxxxxxxx/ >> [2] https://lore.kernel.org/all/20220512074037.3829926-1-benjamin.mugnier@xxxxxxxxxxx/ >> >>>>>>> + >>>>>>> +#endif /* __UAPI_MLX7502X_H_ */ >>>> >>>> -- >>>> Regards, >>>> >>>> Laurent Pinchart >>> >
