Hi Dave, Quoting Dave Stevenson (2022-03-10 15:21:28) > Hi Benjamin and Kieran > > On Thu, 10 Mar 2022 at 13:52, Kieran Bingham > <kieran.bingham@xxxxxxxxxxxxxxxx> wrote: > > > > Hi Benjamin, > > > > Thank you for the patch - > > > > Quoting Benjamin Mugnier (2022-03-10 13:32:55) > > > The VGXY61 has a quad lanes CSI-2 output port running at 800mbps per > > > lane, and supports RAW8, RAW10, RAW12, RAW14 and RAW16 formats. > > > The driver handles both sensor types: > > > - VG5661 and VG6661: 1.6 Mpx (1464 x 1104) 75fps. > > > - VG5761 and VG6761: 2.3 Mpx (1944 x 1204) 60 fps. > > > The driver supports: > > > - HDR linearize mode, HDR substraction mode, and no HDR > > > - GPIOs LEDs strobing > > > - Digital binning and analog subsampling > > > - Horizontal and vertical flip > > > - Manual exposure > > > - Analog and digital gains > > > - Test patterns > > > > I haven't reviewed the driver directly yet, but I have a script which > > looks for key requirements for libcamera. > > (https://git.linuxtv.org/libcamera.git/tree/Documentation/sensor_driver_requirements.rst) > > > > > > Media Controller Support: > > - V4L2_SUBDEV_FL_HAS_DEVNODE : found > > - MEDIA_ENT_F_CAM_SENSOR : found > > > > Mandatory Controls: > > - V4L2_CID_EXPOSURE : found > > - V4L2_CID_HBLANK : -------- > > - V4L2_CID_VBLANK : -------- > > - V4L2_CID_PIXEL_RATE : found > > > > Selection Controls (will become mandatory): > > - V4L2_SEL_TGT_CROP_DEFAULT : -------- > > - V4L2_SEL_TGT_CROP : -------- > > - V4L2_SEL_TGT_CROP_BOUNDS : -------- > > - .get_selection : -------- > > > > Optional Controls: > > - V4L2_CID_TEST_PATTERN : found > > - V4L2_CID_GAIN : -------- > > - V4L2_CID_ANALOGUE_GAIN : found > > - V4L2_CID_CAMERA_SENSOR_ROTATION : -------- > > - V4L2_CID_CAMERA_ORIENTATION : -------- > > > > > > The key missing pieces are HBLANK/VBLANK and the .get_selection API. Is > > it easy/feasible to add these? > > It's documented that HBLANK/VBLANK are the correct parameters to use > for raw image sensors [1]. > > It looks like register DEVICE_FRAME_LENGTH is mode->height + > V4L2_CID_VBLANK, and DEVICE_LINE_LENGTH is mode->width + > V4L2_CID_HBLANK (appears to be treated as read only) > Most other sensors will adjust the max value of V4L2_CID_EXPOSURE > based on the frame length. > > [1] https://www.kernel.org/doc/html/latest/driver-api/media/camera-sensor.html#frame-interval-configuration Aha, Thanks - I hadn't realised it was 'formally' required by the kernel, so I was trying to be more gentle. I'll be more assertive on those now ;-) and I've added the reference to my review script. -- Kieran
