Add documentation for AMD isp 4 and describe the main components
Signed-off-by: Bin Du <Bin.Du@amd.com>
Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
---
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. 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
+
+ # 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@amd.com>
L: linux-media@vger.kernel.org
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/*
--
2.34.1On 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@amd.com>
> Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
> ---
> 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().
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.
> 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.
> +
> + # 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@amd.com>
> L: linux-media@vger.kernel.org
> 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,
Laurent Pinchart
Many thanks Laurent Pinchart for the review.
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@amd.com>
>> Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
>> ---
>> 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.
> 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. 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.
>> 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@amd.com>
>> L: linux-media@vger.kernel.org
>> 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/*
>
Hi Bin, On Tue, Aug 12, 2025 at 09:36:04AM +0800, Du, Bin wrote: > Many thanks Laurent Pinchart for the review. > > 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@amd.com> > > > Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com> > > > --- > > > 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. > > > 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. 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. There are a few problems I see with this approach. Camera sensors are separate devices from the ISP and they're expected to be controlled by the respective camera sensor drivers and these drivers only. The firmware contains the camera control algorithms as well as tuning; this is something that's better located outside of it. The complex camera system comprising of a camera sensor, an ISP and a microcontroller within you have is right now semi-integrated to the SoC and I think it needs to be either fully unintegrated (the ISPs we currently support) or fully integrated (e.g. UVC webcams). There are two options that I can see here, in descending order of preference: 1. Control the ISP processing blocks from the AMD ISP driver, via a documented UAPI. This includes setting processing block parameters and being able to obtain statistics from the ISP. This is aligned with the other currently supported ISP drivers. This option could include support for the CSI-2 receiver only, with the processing taking place in SoftISP. Fully supported ISP would of course be preferred. Right now I don't have an opinion on whether or not this needs to include libcamera support, but the ISP driver wouldn't be of much use without that anyway. 2. Move sensor control to firmware and make the AMD ISP driver expose an interface that looks like a webcam, akin to the UVC driver. In this case there's also no use for the sensor driver you've posted earlier. Overall, the ISP/sensor combination will probably be limited to use as a webcam in this case. -- Kind regards, Sakari Ailus
Many thanks Sakari Ailus for your deep insight On 8/20/2025 8:42 PM, Sakari Ailus wrote: > Hi Bin, > > On Tue, Aug 12, 2025 at 09:36:04AM +0800, Du, Bin wrote: >> Many thanks Laurent Pinchart for the review. >> >> 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@amd.com> >>>> Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com> >>>> --- >>>> 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. >> >>> 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. 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. > > There are a few problems I see with this approach. > > Camera sensors are separate devices from the ISP and they're expected to be > controlled by the respective camera sensor drivers and these drivers only. > The firmware contains the camera control algorithms as well as tuning; this > is something that's better located outside of it. > > The complex camera system comprising of a camera sensor, an ISP and a > microcontroller within you have is right now semi-integrated to the SoC and > I think it needs to be either fully unintegrated (the ISPs we currently > support) or fully integrated (e.g. UVC webcams). > > There are two options that I can see here, in descending order of > preference: > > 1. Control the ISP processing blocks from the AMD ISP driver, via a > documented UAPI. This includes setting processing block parameters and > being able to obtain statistics from the ISP. This is aligned with the > other currently supported ISP drivers. > > This option could include support for the CSI-2 receiver only, with the > processing taking place in SoftISP. Fully supported ISP would of course > be preferred. > > Right now I don't have an opinion on whether or not this needs to > include libcamera support, but the ISP driver wouldn't be of much use > without that anyway. > > 2. Move sensor control to firmware and make the AMD ISP driver expose an > interface that looks like a webcam, akin to the UVC driver. In this case > there's also no use for the sensor driver you've posted earlier. > Overall, the ISP/sensor combination will probably be limited to use as a > webcam in this case. > Based on our internal discussion and validation, will make option 2 as our current upstream target, after that, will plan option 1 with more considerations, a. Whether to provide the capability and interface so user can do switch between option 1 and 2. b. Whether and how to expose interface so host can leverage the ISP HW feature to accelerat some processing. Though sensor driver is not used in option 2, we still plan to upstream it because of option 1 as next step. -- Regards, Bin
Hi Bin, On Fri, Aug 22, 2025 at 10:20:01AM +0800, Du, Bin wrote: > Many thanks Sakari Ailus for your deep insight > > On 8/20/2025 8:42 PM, Sakari Ailus wrote: > > Hi Bin, > > > > On Tue, Aug 12, 2025 at 09:36:04AM +0800, Du, Bin wrote: > > > Many thanks Laurent Pinchart for the review. > > > > > > 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@amd.com> > > > > > Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com> > > > > > --- > > > > > 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. > > > > > > > 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. 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. > > > > There are a few problems I see with this approach. > > > > Camera sensors are separate devices from the ISP and they're expected to be > > controlled by the respective camera sensor drivers and these drivers only. > > The firmware contains the camera control algorithms as well as tuning; this > > is something that's better located outside of it. > > > > The complex camera system comprising of a camera sensor, an ISP and a > > microcontroller within you have is right now semi-integrated to the SoC and > > I think it needs to be either fully unintegrated (the ISPs we currently > > support) or fully integrated (e.g. UVC webcams). > > > > There are two options that I can see here, in descending order of > > preference: > > > > 1. Control the ISP processing blocks from the AMD ISP driver, via a > > documented UAPI. This includes setting processing block parameters and > > being able to obtain statistics from the ISP. This is aligned with the > > other currently supported ISP drivers. > > This option could include support for the CSI-2 receiver only, with the > > processing taking place in SoftISP. Fully supported ISP would of course > > be preferred. > > Right now I don't have an opinion on whether or not this needs to > > include libcamera support, but the ISP driver wouldn't be of much use > > without that anyway. > > > > 2. Move sensor control to firmware and make the AMD ISP driver expose an > > interface that looks like a webcam, akin to the UVC driver. In this case > > there's also no use for the sensor driver you've posted earlier. > > Overall, the ISP/sensor combination will probably be limited to use as a > > webcam in this case. > > > > Based on our internal discussion and validation, will make option 2 as our > current upstream target, after that, will plan option 1 with more > considerations, a. Whether to provide the capability and interface so user > can do switch between option 1 and 2. b. Whether and how to expose interface > so host can leverage the ISP HW feature to accelerat some processing. Though > sensor driver is not used in option 2, we still plan to upstream it because > of option 1 as next step. I expect this to take some time. In the meantime, I'm inclined to merge ov02c05 driver from Bingbu / Hao in case they can provide an upstreamable one as that driver would have users already today. That being said, it won't be a problem accommodating the needs of both into the same driver. -- Kind regards, Sakari Ailus
Many thanks Sakari for the update. All clear on my end — feel free to take the time you need. On 9/22/2025 2:24 PM, Sakari Ailus wrote: > Hi Bin, > > On Fri, Aug 22, 2025 at 10:20:01AM +0800, Du, Bin wrote: >> Many thanks Sakari Ailus for your deep insight >> >> On 8/20/2025 8:42 PM, Sakari Ailus wrote: >>> Hi Bin, >>> >>> On Tue, Aug 12, 2025 at 09:36:04AM +0800, Du, Bin wrote: >>>> Many thanks Laurent Pinchart for the review. >>>> >>>> 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@amd.com> >>>>>> Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com> >>>>>> --- >>>>>> 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. >>>> >>>>> 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. 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. >>> >>> There are a few problems I see with this approach. >>> >>> Camera sensors are separate devices from the ISP and they're expected to be >>> controlled by the respective camera sensor drivers and these drivers only. >>> The firmware contains the camera control algorithms as well as tuning; this >>> is something that's better located outside of it. >>> >>> The complex camera system comprising of a camera sensor, an ISP and a >>> microcontroller within you have is right now semi-integrated to the SoC and >>> I think it needs to be either fully unintegrated (the ISPs we currently >>> support) or fully integrated (e.g. UVC webcams). >>> >>> There are two options that I can see here, in descending order of >>> preference: >>> >>> 1. Control the ISP processing blocks from the AMD ISP driver, via a >>> documented UAPI. This includes setting processing block parameters and >>> being able to obtain statistics from the ISP. This is aligned with the >>> other currently supported ISP drivers. >>> This option could include support for the CSI-2 receiver only, with the >>> processing taking place in SoftISP. Fully supported ISP would of course >>> be preferred. >>> Right now I don't have an opinion on whether or not this needs to >>> include libcamera support, but the ISP driver wouldn't be of much use >>> without that anyway. >>> >>> 2. Move sensor control to firmware and make the AMD ISP driver expose an >>> interface that looks like a webcam, akin to the UVC driver. In this case >>> there's also no use for the sensor driver you've posted earlier. >>> Overall, the ISP/sensor combination will probably be limited to use as a >>> webcam in this case. >>> >> >> Based on our internal discussion and validation, will make option 2 as our >> current upstream target, after that, will plan option 1 with more >> considerations, a. Whether to provide the capability and interface so user >> can do switch between option 1 and 2. b. Whether and how to expose interface >> so host can leverage the ISP HW feature to accelerat some processing. Though >> sensor driver is not used in option 2, we still plan to upstream it because >> of option 1 as next step. > > I expect this to take some time. > > In the meantime, I'm inclined to merge ov02c05 driver from Bingbu / Hao in > case they can provide an upstreamable one as that driver would have users > already today. That being said, it won't be a problem accommodating the > needs of both into the same driver. > -- Regards, Bin
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@amd.com>
> >> Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
> >> ---
> >> 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.
> > 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.
> 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 ?
> >> 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@amd.com>
> >> L: linux-media@vger.kernel.org
> >> 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,
Laurent Pinchart
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@amd.com>
>>>> Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
>>>> ---
>>>> 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@amd.com>
>>>> L: linux-media@vger.kernel.org
>>>> 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
© 2016 - 2026 Red Hat, Inc.