From: Hans de Goede Date: Mon, 29 May 2023 10:37:21 +0000 (+0100) Subject: media: atomisp: Update TODO X-Git-Tag: v6.6.7~2421^2~63 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=36f48c705242bd21a295992c3b0bd2ebbcdef1df;p=platform%2Fkernel%2Flinux-starfive.git media: atomisp: Update TODO A lot of work has been done on the atomisp driver lately. Rewrite the TODO file to drop all the already fixed items: * Moved to videobuf2 + fixed mmap support * Whole bunch of v4l2 API fixes making more apps work * v4l2-async sensor probing support * pm-runtime support (for some sensor drivers at least) * buffer MM code was cleaned up / replaced when moving the videobuf2 And add a new TODO list (retaining some of the old items) split into items which absolutely must be fixed before the driver can be moved out of staging: 1. Conflicting hw-ids with regular sensor drivers 2. Private userspace API stuff As well as a list of items which also definitely needs to be fixed but which could also be fixed after moving the driver out of staging. Link: https://lore.kernel.org/r/20230529103741.11904-2-hdegoede@redhat.com Signed-off-by: Hans de Goede Reviewed-by: Andy Shevchenko Signed-off-by: Mauro Carvalho Chehab --- diff --git a/drivers/staging/media/atomisp/TODO b/drivers/staging/media/atomisp/TODO index 43b8420..12fe1f2 100644 --- a/drivers/staging/media/atomisp/TODO +++ b/drivers/staging/media/atomisp/TODO @@ -1,213 +1,83 @@ -For both Cherrytrail (CHT) and Baytrail (BHT) the driver -requires the "candrpv_0415_20150521_0458" firmware version. -It should be noticed that the firmware file is different, -depending on the ISP model, so they're stored with different -names: +Required firmware +================= -- for BHT: /lib/firmware/shisp_2400b0_v21.bin +The atomisp driver requires the following firmware: - Warning: The driver was not tested yet for BHT. +- for BYT: /lib/firmware/shisp_2400b0_v21.bin -- for CHT: /lib/firmware/shisp_2401a0_v21.bin - - https://github.com/intel-aero/meta-intel-aero-base/blob/master/recipes-kernel/linux/linux-yocto/shisp_2401a0_v21.bin - -NOTE: -===== - -This driver currently doesn't work with most V4L2 applications, -as there are still some issues with regards to implementing -certain APIs at the standard way. - -Also, currently only USERPTR streaming mode is working. + With a version of "irci_stable_candrpv_0415_20150423_1753" to check + the version run: "strings shisp_2400b0_v21.bin | head -n1", sha256sum: -In order to test, it is needed to know what's the sensor's -resolution. This can be checked with: + 3847b95fb9f1f8352c595ba7394d55b33176751372baae17f89aa483ec02a21b shisp_2400b0_v21.bin -$ v4l2-ctl --get-fmt-video - Format Video Capture: - Width/Height : 1600/1200 - ... + The shisp_2400b0_v21.bin file with this version can be found on + the Android factory images of various X86 Android tablets such as + e.g. the Chuwi Hi8 Pro. -It is known to work with: - -- v4l2grab at contrib/test directory at https://git.linuxtv.org/v4l-utils.git/ - - The resolution should not be bigger than the max resolution - supported by the sensor, or it will fail. So, if the sensor - reports: - - The driver can be tested with: - - v4l2grab -f YUYV -x 1600 -y 1200 -d /dev/video2 -u +- for CHT: /lib/firmware/shisp_2401a0_v21.bin -- NVT at https://github.com/intel/nvt + With a version of "irci_stable_candrpv_0415_20150521_0458", sha256sum: - $ ./v4l2n -o testimage_@.raw \ - --device /dev/video2 \ - --input 0 \ - --exposure=30000,30000,30000,30000 \ - --parm type=1,capturemode=CI_MODE_PREVIEW \ - --fmt type=1,width=1600,height=1200,pixelformat=YUYV \ - --reqbufs count=2,memory=USERPTR \ - --parameters=wb_config.r=32768,wb_config.gr=21043,wb_config.gb=21043,wb_config.b=30863 \ - --capture=20 + e89359f4e4934c410c83d525e283f34c5fcce9cb5caa75ad8a32d66d3842d95c shisp_2401a0_v21.bin - As the output is in raw format, images need to be converted with: + This can be found here: + https://github.com/intel-aero/meta-intel-aero-base/blob/master/recipes-kernel/linux/linux-yocto/shisp_2401a0_v21.bin - $ for i in $(seq 0 19); do - name="testimage_$(printf "%03i" $i)" - ./raw2pnm -x$WIDTH -y$HEIGHT -f$FORMAT $name.raw $name.pnm - rm $name.raw - done TODO ==== -1. Fix support for MMAP streaming mode. This is required for most - V4L2 applications; - -2. Implement and/or fix V4L2 ioctls in order to allow a normal app to - use it; - -3. Ensure that the driver will pass v4l2-compliance tests; - -4. Get manufacturer's authorization to redistribute the binaries for - the firmware files; - -5. remove VIDEO_ATOMISP_ISP2401, making the driver to auto-detect the - register address differences between ISP2400 and ISP2401; - -6. Cleanup the driver code, removing the abstraction layers inside it; - -7. The atomisp doesn't rely at the usual i2c stuff to discover the - sensors. Instead, it calls a function from atomisp_gmin_platform.c. - There are some hacks added there for it to wait for sensors to be - probed (with a timeout of 2 seconds or so). This should be converted - to the usual way, using V4L2 async subdev framework to wait for - cameras to be probed; - -8. Switch to standard V4L2 sub-device API for sensor and lens. In - particular, the user space API needs to support V4L2 controls as - defined in the V4L2 spec and references to atomisp must be removed from - these drivers. - -9. Use LED flash API for flash LED drivers such as LM3554 (which already - has a LED class driver). - -10. Migrate the sensor drivers out of staging or re-using existing - drivers; - -11. Switch the driver to use pm_runtime stuff. Right now, it probes the - existing PMIC code and sensors call it directly. - -12. There's a problem on sensor drivers: when trying to set a video - format, the atomisp main driver calls the sensor drivers with the - sensor turned off. This causes them to fail. - - This was fixed at atomisp-ov2880, which has a hack inside it - to turn it on when VIDIOC_S_FMT is called, but this has to be - cheked on other drivers as well. - - The right fix seems to power on the sensor when a video device is - opened (or at the first VIDIOC_ ioctl - except for VIDIOC_QUERYCAP), - powering it down at close() syscall. - - Such kind of control would need to be done inside the atomisp driver, - not at the sensors code. - -13. There are several issues related to memory management, that can - cause crashes and/or memory leaks. The atomisp splits the memory - management on three separate regions: - - - dynamic pool; - - reserved pool; - - generic pool - - The code implementing it is at: - - drivers/staging/media/atomisp/pci/hmm/ - - It also has a separate code for managing DMA buffers at: - - drivers/staging/media/atomisp/pci/mmu/ - - The code there is really dirty, ugly and probably wrong. I fixed - one bug there already, but the best would be to just trash it and use - something else. Maybe the code from the newer intel driver could - serve as a model: - - drivers/staging/media/ipu3/ipu3-mmu.c - - But converting it to use something like that is painful and may - cause some breakages. - -14. The file structure needs to get tidied up to resemble a normal Linux - driver. - -15. Lots of the midlayer glue. Unused code and abstraction needs removing. - -16. The AtomISP driver includes some special IOCTLS (ATOMISP_IOC_XXXX_XXXX) - and controls that require some cleanup. Some of those code may have - been removed during the cleanups. They could be needed in order to - properly support 3A algorithms. - - Such IOCTL interface needs more documentation. The better would - be to use something close to the interface used by the IPU3 IMGU driver. - -17. The ISP code has some dependencies of the exact FW version. - The version defined in pci/sh_css_firmware.c: - - BYT (isp2400): "irci_stable_candrpv_0415_20150521_0458" - - CHT (isp2401): "irci_ecr - master_20150911_0724" - - Those versions don't seem to be available anymore. On the tests we've - done so far, this version also seems to work for CHT: +1. Items which MUST be fixed before the driver can be moved out of staging: - "irci_stable_candrpv_0415_20150521_0458" +* The atomisp ov2680 and ov5693 sensor drivers bind to the same hw-ids as + the standard ov2680 and ov5693 drivers under drivers/media/i2c, which + conflicts. Drop the atomisp private ov2680 and ov5693 drivers: + * Make atomisp code use v4l2 selections to deal with the extra padding + it wants to receive from sensors instead of relying on the ov2680 code + sending e.g. 1616x1216 for a 1600x1200 mode + * Port various ov2680 improvements from atomisp_ov2680.c to regular ov2680.c + and switch to regular ov2680 driver + * Make atomisp work with the regular ov5693 driver and drop atomisp_ov5693 - Which can be obtainable from Yocto Atom ISP respository. +* Fix atomisp causing the whole machine to hang in its probe() error-exit + path taken in the firmware missing case - but this was not thoroughly tested. +* Remove/disable private IOCTLs - At some point we may need to round up a few driver versions and see if - there are any specific things that can be done to fold in support for - multiple firmware versions. +* Remove/disable custom v4l2-ctrls +* Remove custom sysfs files created by atomisp_drvfs.c -18. Switch from videobuf1 to videobuf2. Videobuf1 is being removed! +* Remove abuse of priv field in various v4l2 userspace API structs -19. Correct Coding Style. Please refrain sending coding style patches - for this driver until the other work is done, as there will be a lot - of code churn until this driver becomes functional again. +* Without a 3A library the capture behaviour is not very good. To take a good + picture, the exposure/gain needs to be tuned using v4l2-ctl on the sensor + subdev. To fix this, support for the atomisp needs to be added to libcamera. -20. Remove the logic which sets up pipelines inside it, moving it to - libcamera and implement MC support. + This MUST be done before moving the driver out of staging so that we can + still make changes to e.g. the mediactl topology if necessary for + libcamera integration. Since this would be a userspace API break, this + means that at least proof-of-concept libcamera integration needs to be + ready before moving the driver out of staging. -Limitations -=========== +2. Items which SHOULD also be fixed eventually: -1. To test the patches, you also need the ISP firmware +* Remove VIDEO_ATOMISP_ISP2401, making the driver to auto-detect the + register address differences between ISP2400 and ISP2401 - for BYT: /lib/firmware/shisp_2400b0_v21.bin - for CHT: /lib/firmware/shisp_2401a0_v21.bin +* The driver is intended to drive the PCI exposed versions of the device. + It will not detect those devices enumerated via ACPI as a field of the + i915 GPU driver (only a problem on BYT). - The firmware files will usually be found in /etc/firmware on an Android - device but can also be extracted from the upgrade kit if you've managed - to lose them somehow. + There are some patches adding i915 GPU support floating at the Yocto's + Aero repository (so far, untested upstream). -2. Without a 3A library the capture behaviour is not very good. To take a good - picture, you need tune ISP parameters by IOCTL functions or use a 3A library - such as libxcam. +* Ensure that the driver will pass v4l2-compliance tests -3. The driver is intended to drive the PCI exposed versions of the device. - It will not detect those devices enumerated via ACPI as a field of the - i915 GPU driver. +* Fix not all v4l2 apps working, e.g. cheese does not work - There are some patches adding i915 GPU support floating at the Yocto's - Aero repository (so far, untested upstream). +* Get manufacturer's authorization to redistribute the binaries for + the firmware files -4. The driver supports only v2 of the IPU/Camera. It will not work with the - versions of the hardware in other SoCs. +* The atomisp code still has a lot of cruft which needs cleaning up