From: Hans de Goede <hdegoede@redhat.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
Andy Shevchenko <andy@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>, Kate Hsuan <hpa@redhat.com>,
Tsuchiya Yuto <kitakar@gmail.com>,
Yury Luneff <yury.lunev@gmail.com>,
Nable <nable.maininbox@googlemail.com>,
andrey.i.trufanov@gmail.com, Fabio Aiuto <fabioaiuto83@gmail.com>,
linux-media@vger.kernel.org, linux-staging@lists.linux.dev
Subject: [PATCH 01/21] media: atomisp: Update TODO
Date: Mon, 29 May 2023 12:37:21 +0200 [thread overview]
Message-ID: <20230529103741.11904-2-hdegoede@redhat.com> (raw)
In-Reply-To: <20230529103741.11904-1-hdegoede@redhat.com>
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.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/media/atomisp/TODO | 236 +++++++----------------------
1 file changed, 51 insertions(+), 185 deletions(-)
diff --git a/drivers/staging/media/atomisp/TODO b/drivers/staging/media/atomisp/TODO
index 43b842043f29..5e7bb6eb351a 100644
--- a/drivers/staging/media/atomisp/TODO
+++ b/drivers/staging/media/atomisp/TODO
@@ -1,213 +1,79 @@
-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
+
+ With a version of "irci_stable_candrpv_0415_20150423_1753" to check
+ the version run: "strings shisp_2400b0_v21.bin | head -n1" .
+
+ 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.
- for CHT: /lib/firmware/shisp_2401a0_v21.bin
+ With a version of "irci_stable_candrpv_0415_20150521_0458"
+
+ 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
-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.
-
-In order to test, it is needed to know what's the sensor's
-resolution. This can be checked with:
-
-$ v4l2-ctl --get-fmt-video
- Format Video Capture:
- Width/Height : 1600/1200
- ...
-
-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
-
-- NVT at https://github.com/intel/nvt
-
- $ ./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
-
- As the output is in raw format, images need to be converted with:
-
- $ 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;
+1. items which MUST be fixed before the driver can be moved out of staging:
-2. Implement and/or fix V4L2 ioctls in order to allow a normal app to
- use it;
+* 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
-3. Ensure that the driver will pass v4l2-compliance tests;
+* Fix atomisp causing the whole machine to hang in its probe() error-exit
+ path taken in the firmware missing case
-4. Get manufacturer's authorization to redistribute the binaries for
- the firmware files;
+* Remove/disable private ioctls
-5. remove VIDEO_ATOMISP_ISP2401, making the driver to auto-detect the
- register address differences between ISP2400 and ISP2401;
+* Remove/disable custom v4l2-ctrls
-6. Cleanup the driver code, removing the abstraction layers inside it;
+* Remove custom sysfs files created by atomisp_drvfs.c
-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;
+* Remove abuse of priv field in various v4l2 userspace API structs
-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.
+* 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.
-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:
-
- "irci_stable_candrpv_0415_20150521_0458"
-
- Which can be obtainable from Yocto Atom ISP respository.
-
- but this was not thoroughly tested.
-
- 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.
+ 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.
-18. Switch from videobuf1 to videobuf2. Videobuf1 is being removed!
+2. items which SHOULD also be fixed eventually:
-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.
+* Remove VIDEO_ATOMISP_ISP2401, making the driver to auto-detect the
+ register address differences between ISP2400 and ISP2401
-20. Remove the logic which sets up pipelines inside it, moving it to
- libcamera and implement MC support.
+* 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).
+ There are some patches adding i915 GPU support floating at the Yocto's
+ Aero repository (so far, untested upstream).
-Limitations
-===========
+* Ensure that the driver will pass v4l2-compliance tests
-1. To test the patches, you also need the ISP firmware
+* Fix not all v4l2 apps working, e.g. cheese does not work
- for BYT: /lib/firmware/shisp_2400b0_v21.bin
- for CHT: /lib/firmware/shisp_2401a0_v21.bin
+* Get manufacturer's authorization to redistribute the binaries for
+ the firmware files
- 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.
-
-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.
-
-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.
-
- There are some patches adding i915 GPU support floating at the Yocto's
- Aero repository (so far, untested upstream).
-
-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
--
2.40.1
next prev parent reply other threads:[~2023-05-29 10:38 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-29 10:37 [PATCH 00/21] media: atomisp: Use selection API info to determine sensor padding Hans de Goede
2023-05-29 10:37 ` Hans de Goede [this message]
2023-05-29 21:57 ` [PATCH 01/21] media: atomisp: Update TODO Andy Shevchenko
2023-05-29 10:37 ` [PATCH 02/21] media: atomisp: ov2680: s/ov2680_device/ov2680_dev/ Hans de Goede
2023-05-29 10:37 ` [PATCH 03/21] media: atomisp: ov2680: s/input_lock/lock/ Hans de Goede
2023-05-29 10:37 ` [PATCH 04/21] media: atomisp: ov2680: Add missing ov2680_calc_mode() call to probe() Hans de Goede
2023-05-29 10:37 ` [PATCH 05/21] media: atomisp: ov2680: Add init_cfg pad-op Hans de Goede
2023-05-29 18:13 ` Andy Shevchenko
2023-05-30 11:51 ` Andy Shevchenko
2023-05-31 11:11 ` Hans de Goede
2023-05-29 10:37 ` [PATCH 06/21] media: atomisp: ov2680: Implement selection support Hans de Goede
2023-05-29 20:31 ` Andy Shevchenko
2023-05-30 10:36 ` Hans de Goede
2023-05-30 11:28 ` Andy Shevchenko
2023-05-30 14:17 ` Hans de Goede
2023-05-29 10:37 ` [PATCH 07/21] media: atomisp: Remove a bunch of sensor related custom IOCTLs Hans de Goede
2023-05-29 10:37 ` [PATCH 08/21] media: atomisp: Remove redundant atomisp_subdev_set_selection() calls from atomisp_set_fmt() Hans de Goede
2023-05-29 10:37 ` [PATCH 09/21] media: atomisp: Simplify atomisp_subdev_set_selection() calls in atomisp_set_fmt() Hans de Goede
2023-05-29 10:37 ` [PATCH 10/21] media: atomisp: Add target validation to atomisp_subdev_set_selection() Hans de Goede
2023-05-29 10:37 ` [PATCH 11/21] media: atomisp: Remove bogus fh use from atomisp_set_fmt*() Hans de Goede
2023-05-29 10:37 ` [PATCH 12/21] media: atomisp: Add input helper variable for isp->asd->inputs[asd->input_curr] Hans de Goede
2023-05-29 10:37 ` [PATCH 13/21] media: atomisp: Add ia_css_frame_pad_width() helper function Hans de Goede
2023-05-29 20:57 ` Andy Shevchenko
2023-05-30 10:43 ` Hans de Goede
2023-05-29 10:37 ` [PATCH 14/21] media: atomisp: Refactor atomisp_try_fmt() / atomisp_set_fmt() Hans de Goede
2023-05-29 21:05 ` Andy Shevchenko
2023-05-30 11:58 ` Andy Shevchenko
2023-05-31 11:28 ` Hans de Goede
2023-05-29 10:37 ` [PATCH 15/21] media: atomisp: Add support for sensors which implement selection API / cropping Hans de Goede
2023-05-29 10:37 ` [PATCH 16/21] media: atomisp: Pass MEDIA_BUS_FMT_* code when calling enum_frame_size pad-op Hans de Goede
2023-05-29 10:37 ` [PATCH 17/21] media: atomisp: Make atomisp_init_sensor() check if the sensor supports binning Hans de Goede
2023-05-29 10:37 ` [PATCH 18/21] media: atomisp: Use selection API info to determine sensor padding Hans de Goede
2023-05-29 10:37 ` [PATCH 19/21] media: atomisp: Set crop before setting fmt Hans de Goede
2023-05-29 10:37 ` [PATCH 20/21] media: atomisp: Add enum_framesizes function for sensors with selection / crop support Hans de Goede
2023-05-29 10:37 ` [PATCH 21/21] media: atomisp: csi2-bridge: Set PMC clk-rate for sensors to 19.2 MHz Hans de Goede
2023-05-29 21:48 ` Andy Shevchenko
2023-05-30 10:28 ` Hans de Goede
2023-05-30 11:32 ` Andy Shevchenko
2023-05-29 21:59 ` [PATCH 00/21] media: atomisp: Use selection API info to determine sensor padding Andy Shevchenko
2023-05-30 10:55 ` Hans de Goede
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230529103741.11904-2-hdegoede@redhat.com \
--to=hdegoede@redhat.com \
--cc=andrey.i.trufanov@gmail.com \
--cc=andy@kernel.org \
--cc=fabioaiuto83@gmail.com \
--cc=hpa@redhat.com \
--cc=kitakar@gmail.com \
--cc=linux-media@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=mchehab@kernel.org \
--cc=nable.maininbox@googlemail.com \
--cc=sakari.ailus@linux.intel.com \
--cc=yury.lunev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox