Commit 36f48c70 authored by Hans de Goede's avatar Hans de Goede Committed by Mauro Carvalho Chehab

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.comSigned-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Reviewed-by: default avatarAndy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@kernel.org>
parent fadac6af
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment