Hi Hans, On Sun, Jan 21, 2018 at 11:46:46AM +0100, Hans Verkuil wrote: > Hi Sakari, > > I looked a bit closer at how atomisp uses g/s_parm. They abuse the capturemode field > to select video/preview/still modes on the sensor, which actually changes the list > of supported resolutions. > > The following files use this: > > i2c/atomisp-gc0310.c > i2c/atomisp-gc2235.c > i2c/atomisp-ov2680.c > i2c/atomisp-ov2722.c > i2c/ov5693/atomisp-ov5693.c > pci/atomisp2/atomisp_file.c > pci/atomisp2/atomisp_tpg.c > > The last two have a dummy g/s_parm implementation, so are easy to fix. > The gc0310 and 0v2680 have identical resolution lists for all three modes, so > the capturemode can just be ignored and these two drivers can be simplified. > > Looking at the higher level code it turns out that this atomisp driver appears > to be in the middle of a conversion from using s_parm to a V4L2_CID_RUN_MODE > control. If the control is present, then that will be used to set the mode, > otherwise it falls back to s_parm. > > So the best solution would be if Intel can convert the remaining drivers from > using s_parm to the new control and then drop all code that uses s_parm to do > this, so g/s_parm is then only used to get/set the framerate. > > Is this something you or a colleague can take on? I've stabbed the atomisp sensor drivers enough to remove the s_parm and g_parm usage there. This effectively removes the s_parm abuse, as there was nothing else it was being used for. The patches are here; there are no changes to your patches in the branch you pointed me to: <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=sparm> I've split dropping support for certain modes in the drivers into separate patch; it's easy to bring them back by just reverting the patch ("staging: atomisp: i2c: Disable non-preview configurations") or removing the ifdefs. I don't object merging this with the previous patch either. What comes to the run mode control --- this logic should have always resided in user space; that control (and s_parm hack) is basically getting around lack of support for MC / V4L2 sub-device interface in the driver. So that control isn't the right solution either going forward. Cc Andy and Alan. -- Regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx
