Re: [PATCH v2 8/8] Documentation: add documentation of AMD isp 4 driver

From: Du, Bin
Date: Thu Aug 21 2025 - 22:29:06 EST

Many thanks Laurent, sorry for the late response because we had a lot of internal discussion and did possible solution prototype validation.

On 8/12/2025 9:42 PM, Laurent Pinchart wrote:
On Tue, Aug 12, 2025 at 09:36:04AM +0800, Du, Bin wrote:
On 8/5/2025 7:37 PM, Laurent Pinchart wrote:
On Wed, Jun 18, 2025 at 05:19:59PM +0800, Bin Du wrote:
Add documentation for AMD isp 4 and describe the main components

Signed-off-by: Bin Du <Bin.Du@xxxxxxx>
Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@xxxxxxx>
---
Documentation/admin-guide/media/amdisp4-1.rst | 64 +++++++++++++++++++
Documentation/admin-guide/media/amdisp4.dot | 8 +++
MAINTAINERS | 2 +
3 files changed, 74 insertions(+)
create mode 100644 Documentation/admin-guide/media/amdisp4-1.rst
create mode 100644 Documentation/admin-guide/media/amdisp4.dot

diff --git a/Documentation/admin-guide/media/amdisp4-1.rst b/Documentation/admin-guide/media/amdisp4-1.rst
new file mode 100644
index 000000000000..417b15af689a
--- /dev/null
+++ b/Documentation/admin-guide/media/amdisp4-1.rst
@@ -0,0 +1,64 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. include:: <isonum.txt>
+
+====================================
+AMD Image Signal Processor (amdisp4)
+====================================
+
+Introduction
+============
+
+This file documents the driver for the AMD ISP4 that is part of
+AMD Ryzen AI Max 385 SoC.
+
+The driver is located under drivers/media/platform/amd/isp4 and uses
+the Media-Controller API.
+
+Topology
+========
+
+.. _amdisp4_topology_graph:
+
+.. kernel-figure:: amdisp4.dot
+ :alt: Diagram of the media pipeline topology
+ :align: center
+
+
+
+The driver has 1 sub-device:
+
+- isp: used to resize and process bayer raw frames in to yuv.
+
+The driver has 1 video device:
+
+- <capture video device: capture device for retrieving images.
+
+
+ - ISP4 Image Signal Processing Subdevice Node
+-----------------------------------------------
+
+The isp4 is represented as a single V4L2 subdev, the sub-device does not
+provide interface to the user space.

Doesn't it ? The driver sets the V4L2_SUBDEV_FL_HAS_DEVNODE flag for the
subdev, and calls v4l2_device_register_subdev_nodes().

We have exported subdev device to user space during the testing with
libcamera sample pipeline.

But it's not needed anymore ? If not, you could stop exposing the subdev
to userspace for the time being.


Yes, will not expose ISP subdev to userspace. Please see more details below.

As far as I understand, the camera is exposed by the firmware with a
webcam-like interface. We need to better understand your plans with this
driver. If everything is handled by the firmware, why are the sensor and
subdev exposed to userspace ? Why can't you expose a single video
capture device, with a media device, and handle everything behind the
scene ? I assume there may be more features coming later. Please
document the plan, we can't provide feedback on the architecture
otherwise.

Currently, isp fw is controlling the sensor to update just the exposure
and gain, since the 3A algorithms run on ISP HW rather than on x86.

This design decision makes my hair stand on end :-( Exposing the camera
sensor to both the firmware and the host concurrently is asking for
trouble. If you really want to abstract the camera behind a firmware and
only expose a webcam-like API (or not even that in this version, as the
driver exposes no control as far as I can see), then you should push the
whole sensor handling to the firmware too. In my opinion that would not
be a good solution compared to exposing the ISP to the host, but it
would be better than this hybrid model.


Totally agree, yes, the hybrid model is really a bad idea, based on our internal validation, we plan to take one of your suggestions, that is to push the whole sensor handling to the firmware, so our ISP driver will work like UVC driver.

In a
future version, we plan to introduce raw output support in the ISP
driver, allowing users to choose between AMD’s 3A running on ISP
hardware or a custom 3A running on x86. If the user opts for the
x86-based 3A, the firmware will relinquish control of the sensor, and
hands over full control to the x86 system.

Will the firmware at that point expose to Linux all the ISP statistics
needed to implement auto-exposure ? What if I want to also set digital
gain ? Or have manual white balance (requiring statistics), and manual
CCM ?


No, because all 3A, ISP and camera control will be put inside firmware. Currently, we only support auto mode, so, no manual control will be exposed

The sub-device is connected to one video node
+(isp4_capture) with immutable active link. The isp entity is connected
+to sensor pad 0 and receives the frames using CSI-2 protocol. The sub-device is
+also responsible to configure CSI2-2 receiver.
+The sub-device processes bayer raw data from the connected sensor and output
+them to different YUV formats. The isp also has scaling capabilities.
+
+ - isp4_capture - Frames Capture Video Node
+--------------------------------------------
+
+Isp4_capture is a capture device to capture frames to memory.
+This entity is the DMA engine that write the frames to memory.
+The entity is connected to isp4 sub-device.
+
+Capturing Video Frames Example
+==============================
+
+.. code-block:: bash
+
+ # set the links

This seems very under-documented.

Yes, documentation needs to be updated.

+
+ # start streaming:
+ v4l2-ctl "-d" "/dev/video0" "--set-fmt-video=width=1920,height=1080,pixelformat=NV12" "--stream-mmap" "--stream-count=10"
diff --git a/Documentation/admin-guide/media/amdisp4.dot b/Documentation/admin-guide/media/amdisp4.dot
new file mode 100644
index 000000000000..a4c2f0cceb30
--- /dev/null
+++ b/Documentation/admin-guide/media/amdisp4.dot
@@ -0,0 +1,8 @@
+digraph board {
+ rankdir=TB
+ n00000001 [label="{{<port0> 0} | amd isp4\n | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
+ n00000001:port1 -> n00000004 [style=bold]
+ n00000004 [label="Preview\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
+ n0000000a [label="{{} | ov05c10 22-0010\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
+ n0000000a:port0 -> n00000001:port0 [style=bold]
+}
diff --git a/MAINTAINERS b/MAINTAINERS
index 15070afb14b5..e4455bde376f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1113,6 +1113,8 @@ M: Nirujogi Pratap <pratap.nirujogi@xxxxxxx>
L: linux-media@xxxxxxxxxxxxxxx
S: Maintained
T: git git://linuxtv.org/media.git
+F: Documentation/admin-guide/media/amdisp4-1.rst
+F: Documentation/admin-guide/media/amdisp4.dot
F: drivers/media/platform/amd/Kconfig
F: drivers/media/platform/amd/Makefile
F: drivers/media/platform/amd/isp4/*


--
Regards,
Bin