diff --git a/Documentation/driver-api/media/drivers/apple-isp.rst b/Documentation/driver-api/media/drivers/apple-isp.rst new file mode 100644 index 00000000000000..7eb44e90c5d9cf --- /dev/null +++ b/Documentation/driver-api/media/drivers/apple-isp.rst @@ -0,0 +1,20 @@ +.. SPDX-License-Identifier: GPL-2.0 + +Apple ISP sensor tuning +======================= + +Stream setup loads model-wide sensor tuning using ``CH_SET_FILE_LOAD`` +(0x0111). A missing file allows streaming with a warning. An invalid file, +interrupted read or failed firmware upload fails stream setup instead of +silently continuing. A debug message identifies a successful upload without +dumping its contents. + +Sensor setfiles are distinct from individual camera-module factory +calibration (iCCl). The driver has no iCCl parser, serial binding or upload +command. No factory record or vendor payload belongs in the kernel source +or a distributed firmware package. The separate ANE video denoiser is not +implemented by this driver. + +Command submission alone does not establish the resulting noise reduction +or qualify a firmware metadata layout. In particular, these error-handling +changes do not qualify J613 firmware 26.6.2 preview or its image quality. diff --git a/Documentation/driver-api/media/drivers/index.rst b/Documentation/driver-api/media/drivers/index.rst index 7f6f3dcd5c90c1..1b9def223ec826 100644 --- a/Documentation/driver-api/media/drivers/index.rst +++ b/Documentation/driver-api/media/drivers/index.rst @@ -12,6 +12,7 @@ Video4Linux (V4L) drivers .. toctree:: :maxdepth: 5 + apple-isp bttv-devel cx2341x-devel cx88-devel diff --git a/drivers/media/platform/apple/isp/Kconfig b/drivers/media/platform/apple/isp/Kconfig index b51d19ee115ded..5da72db5a2b374 100644 --- a/drivers/media/platform/apple/isp/Kconfig +++ b/drivers/media/platform/apple/isp/Kconfig @@ -14,9 +14,11 @@ config VIDEO_APPLE_ISP Driver for the image signal processor that runs the built-in camera of Apple silicon machines. The ISP firmware is loaded by the bootloader. When a stream starts, the driver loads the - calibration file for the detected camera module, + sensor tuning file for the detected camera model, apple/isp__XX.dat, from the firmware search - path; if it is missing, the stream runs without calibration. + path; if it is missing, the stream runs without sensor tuning. + Invalid files and firmware upload errors fail the stream setup. + These setfiles do not contain per-module factory calibration. config VIDEO_APPLE_ISP_KUNIT_TEST bool "KUnit tests for the Apple ISP driver" if !KUNIT_ALL_TESTS @@ -24,8 +26,8 @@ config VIDEO_APPLE_ISP_KUNIT_TEST depends on KUNIT=y || VIDEO_APPLE_ISP=m default KUNIT_ALL_TESTS help - Builds unit tests for the videobuf2 queue set-up and for the - shared memory the firmware frees into the Apple ISP driver. They + Builds unit tests for sensor tuning setup, the videobuf2 queue + and shared memory the firmware frees into the Apple ISP driver. They need no hardware. Only useful for kernel developers running KUnit. If unsure, say N. diff --git a/drivers/media/platform/apple/isp/isp-cam-test.c b/drivers/media/platform/apple/isp/isp-cam-test.c new file mode 100644 index 00000000000000..6ed2cb39df22f5 --- /dev/null +++ b/drivers/media/platform/apple/isp/isp-cam-test.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Setfile setup boundary tests; no firmware data or hardware access. */ +#include +#include +#include +#include "isp-ipc.h" + +struct isp_test_command_trace { + unsigned int count; + unsigned int fail_at; + int setfile_error; + unsigned int setfile_calls; +}; + +static int isp_test_send(struct apple_isp *isp, struct isp_channel *chan, + unsigned long timeout) +{ + struct kunit *test = kunit_get_current_test(); + struct isp_test_command_trace *trace = test->priv; + u32 *words = isp->cmd_virt; + + trace->count++; + KUNIT_EXPECT_EQ(test, words[1], trace->count == 1 ? + CISP_CMD_FLICKER_SENSOR_SET : CISP_CMD_CH_SBS_ENABLE); + return trace->count == trace->fail_at ? -ETIME : 0; +} + +static struct apple_isp *isp_test_capture_alloc(struct kunit *test) +{ + static const struct apple_isp_hw hw = { }; + struct apple_isp *isp = kunit_kzalloc(test, sizeof(*isp), GFP_KERNEL); + + KUNIT_ASSERT_NOT_NULL(test, isp); + isp->dev = kunit_device_register(test, "apple-isp-setfile-test"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, isp->dev); + isp->hw = &hw; + isp->chan_io = kunit_kzalloc(test, sizeof(*isp->chan_io), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, isp->chan_io); + isp->chan_io->name = "test"; + isp->cmd_virt = kunit_kzalloc(test, 256, GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, isp->cmd_virt); + kunit_activate_static_stub(test, ipc_chan_send, isp_test_send); + return isp; +} + +static int isp_test_load_setfile(struct apple_isp *isp, u32 ch) +{ + struct kunit *test = kunit_get_current_test(); + struct isp_test_command_trace *trace = test->priv; + + trace->setfile_calls++; + return trace->setfile_error; +} + +static void isp_test_setfile_failure_stops_setup(struct kunit *test) +{ + static const int errors[] = { -EINVAL, -EIO, -ETIME, -EINTR, -ENOMEM }; + struct apple_isp *isp = isp_test_capture_alloc(test); + struct isp_test_command_trace trace; + + kunit_activate_static_stub(test, isp_ch_load_setfile, isp_test_load_setfile); + test->priv = &trace; + for (unsigned int i = 0; i < ARRAY_SIZE(errors); i++) { + trace = (struct isp_test_command_trace){ .setfile_error = errors[i] }; + KUNIT_EXPECT_EQ(test, isp_ch_configure_capture(isp, 0), errors[i]); + /* Only the flicker command preceded the rejected tuning. */ + KUNIT_EXPECT_EQ(test, trace.count, 1); + KUNIT_EXPECT_EQ(test, trace.setfile_calls, 1); + } +} + +static void isp_test_optional_setfile_continues_setup(struct kunit *test) +{ + static const int results[] = { 0, -ENOENT }; + struct apple_isp *isp = isp_test_capture_alloc(test); + struct isp_test_command_trace trace; + + kunit_activate_static_stub(test, isp_ch_load_setfile, isp_test_load_setfile); + test->priv = &trace; + for (unsigned int i = 0; i < ARRAY_SIZE(results); i++) { + trace = (struct isp_test_command_trace){ + .setfile_error = results[i], .fail_at = 2, + }; + /* The SBS command after tuning, rather than tuning, fails. */ + KUNIT_EXPECT_EQ(test, isp_ch_configure_capture(isp, 0), -ETIME); + KUNIT_EXPECT_EQ(test, trace.count, 2); + KUNIT_EXPECT_EQ(test, trace.setfile_calls, 1); + } +} + +static struct kunit_case isp_cam_test_cases[] = { + KUNIT_CASE(isp_test_setfile_failure_stops_setup), + KUNIT_CASE(isp_test_optional_setfile_continues_setup), + {} +}; + +static struct kunit_suite isp_cam_test_suite = { + .name = "apple-isp-cam", + .test_cases = isp_cam_test_cases, +}; + +kunit_test_suite(isp_cam_test_suite); diff --git a/drivers/media/platform/apple/isp/isp-cam.c b/drivers/media/platform/apple/isp/isp-cam.c index 5090ed2ed5b985..c7d619d9b2c637 100644 --- a/drivers/media/platform/apple/isp/isp-cam.c +++ b/drivers/media/platform/apple/isp/isp-cam.c @@ -3,6 +3,7 @@ #include #include +#include #include "isp-cam.h" #include "isp-cmd.h" @@ -343,14 +344,20 @@ static int isp_ch_load_setfile(struct apple_isp *isp, u32 ch) u32 magic; int err; + KUNIT_STATIC_STUB_REDIRECT(isp_ch_load_setfile, isp, ch); + if (WARN_ON_ONCE(setfile->size > isp->data_surf->size)) return -ENOSPC; err = firmware_request_nowarn(&fw, setfile->path, isp->dev); if (err) { - dev_warn_once(isp->dev, - "setfile '%s' not loaded (%d), streaming without calibration\n", - setfile->path, err); + if (err == -ENOENT) + dev_warn_once(isp->dev, + "setfile '%s' missing, streaming without sensor tuning\n", + setfile->path); + else + dev_err(isp->dev, "failed to read setfile '%s': %d\n", + setfile->path, err); return err; } @@ -373,8 +380,16 @@ static int isp_ch_load_setfile(struct apple_isp *isp, u32 ch) memset(isp->data_surf->virt + len, 0, setfile->size - len); release_firmware(fw); - return isp_cmd_ch_set_file_load(isp, ch, isp->data_surf->iova, - setfile->size); + err = isp_cmd_ch_set_file_load(isp, ch, isp->data_surf->iova, + setfile->size); + if (err) + dev_err(isp->dev, "ch %u: setfile '%s' load failed: %d\n", + ch, setfile->path, err); + else + dev_dbg(isp->dev, "ch %u: loaded sensor tuning '%s' (0x%zx bytes)\n", + ch, setfile->path, setfile->size); + + return err; } static int isp_ch_configure_frame_rate(struct apple_isp *isp, u32 ch) @@ -432,11 +447,12 @@ static int isp_ch_configure_capture(struct apple_isp *isp, u32 ch) isp_cmd_flicker_sensor_set(isp, 0); /* - * The setfile isn't requisite but then we don't get calibration. If - * loading it was interrupted by a signal, give up on the stream. + * Missing sensor tuning is optional. A present but invalid setfile or + * a failed firmware upload must not silently select firmware defaults. + * This model-wide tuning is separate from per-module factory data. */ err = isp_ch_load_setfile(isp, ch); - if (err == -EINTR) + if (err && err != -ENOENT) return err; if (isp->hw->lpdp) { @@ -661,3 +677,7 @@ void apple_isp_stop_capture(struct apple_isp *isp) /* ... until its buffers are back. */ apple_isp_wdt_stop(isp); } + +#if IS_ENABLED(CONFIG_VIDEO_APPLE_ISP_KUNIT_TEST) +#include "isp-cam-test.c" +#endif diff --git a/drivers/media/platform/apple/isp/isp-ipc.c b/drivers/media/platform/apple/isp/isp-ipc.c index f709f6e3057639..5ea91e2f4985f8 100644 --- a/drivers/media/platform/apple/isp/isp-ipc.c +++ b/drivers/media/platform/apple/isp/isp-ipc.c @@ -1,6 +1,8 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright 2023 Eileen Yoon */ +#include + #include "isp-iommu.h" #include "isp-ipc.h" #include "isp-regs.h" @@ -176,6 +178,8 @@ int ipc_chan_send(struct apple_isp *isp, struct isp_channel *chan, { long t; + KUNIT_STATIC_STUB_REDIRECT(ipc_chan_send, isp, chan, timeout); + chan_write_msg(isp, chan, &chan->req); dma_wmb();