| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aUkRQEeAWDeQknP6@kekkonen.localdomain>
Date: Mon, 22 Dec 2025 11:37:04 +0200
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Bin Du <Bin.Du@....com>
Cc: mchehab@...nel.org, hverkuil@...all.nl,
laurent.pinchart+renesas@...asonboard.com,
bryan.odonoghue@...aro.org, prabhakar.mahadev-lad.rj@...renesas.com,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
sultan@...neltoast.com, pratap.nirujogi@....com,
benjamin.chan@....com, king.li@....com, gjorgji.rosikopulos@....com,
Phil.Jawich@....com, Dominic.Antony@....com,
mario.limonciello@....com, richard.gong@....com, anson.tsao@....com,
Svetoslav Stoilov <Svetoslav.Stoilov@....com>,
Alexey Zagorodnikov <xglooom@...il.com>
Subject: Re: [PATCH v7 3/7] media: platform: amd: Add isp4 fw and hw interface
Hi Bin,
On Tue, Dec 16, 2025 at 05:13:22PM +0800, Bin Du wrote:
> ISP firmware controls ISP HW pipeline using dedicated embedded processor
> called ccpu. The communication between ISP FW and driver is using commands
> and response messages sent through the ring buffer. Command buffers support
> either global setting that is not specific to the steam and support stream
> specific parameters. Response buffers contain ISP FW notification
> information such as frame buffer done and command done. IRQ is used for
> receiving response buffer from ISP firmware, which is handled in the main
> isp4 media device. ISP ccpu is booted up through the firmware loading
> helper function prior to stream start. Memory used for command buffer and
> response buffer needs to be allocated from amdgpu buffer manager because
> isp4 is a child device of amdgpu.
>
> Co-developed-by: Sultan Alsawaf <sultan@...neltoast.com>
> Signed-off-by: Sultan Alsawaf <sultan@...neltoast.com>
> Co-developed-by: Svetoslav Stoilov <Svetoslav.Stoilov@....com>
> Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@....com>
> Signed-off-by: Bin Du <Bin.Du@....com>
> Tested-by: Alexey Zagorodnikov <xglooom@...il.com>
> ---
> MAINTAINERS | 3 +
> drivers/media/platform/amd/isp4/Makefile | 3 +-
> .../platform/amd/isp4/isp4_fw_cmd_resp.h | 314 +++++++
> .../media/platform/amd/isp4/isp4_interface.c | 786 ++++++++++++++++++
> .../media/platform/amd/isp4/isp4_interface.h | 141 ++++
> 5 files changed, 1246 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/platform/amd/isp4/isp4_fw_cmd_resp.h
> create mode 100644 drivers/media/platform/amd/isp4/isp4_interface.c
> create mode 100644 drivers/media/platform/amd/isp4/isp4_interface.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7aa17c7e71d6..cccae369c876 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1145,7 +1145,10 @@ F: drivers/media/platform/amd/isp4/Kconfig
> F: drivers/media/platform/amd/isp4/Makefile
> F: drivers/media/platform/amd/isp4/isp4.c
> F: drivers/media/platform/amd/isp4/isp4.h
> +F: drivers/media/platform/amd/isp4/isp4_fw_cmd_resp.h
> F: drivers/media/platform/amd/isp4/isp4_hw_reg.h
> +F: drivers/media/platform/amd/isp4/isp4_interface.c
> +F: drivers/media/platform/amd/isp4/isp4_interface.h
>
> AMD KFD
> M: Felix Kuehling <Felix.Kuehling@....com>
> diff --git a/drivers/media/platform/amd/isp4/Makefile b/drivers/media/platform/amd/isp4/Makefile
> index de0092dad26f..a2a5bf98e912 100644
> --- a/drivers/media/platform/amd/isp4/Makefile
> +++ b/drivers/media/platform/amd/isp4/Makefile
> @@ -3,4 +3,5 @@
> # Copyright (C) 2025 Advanced Micro Devices, Inc.
>
> obj-$(CONFIG_AMD_ISP4) += amd_capture.o
> -amd_capture-objs := isp4.o
> +amd_capture-objs := isp4.o \
> + isp4_interface.o
> diff --git a/drivers/media/platform/amd/isp4/isp4_fw_cmd_resp.h b/drivers/media/platform/amd/isp4/isp4_fw_cmd_resp.h
> new file mode 100644
> index 000000000000..d571b3873edb
> --- /dev/null
> +++ b/drivers/media/platform/amd/isp4/isp4_fw_cmd_resp.h
> @@ -0,0 +1,314 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _ISP4_CMD_RESP_H_
> +#define _ISP4_CMD_RESP_H_
> +
> +/*
> + * @brief Host and Firmware command & response channel.
> + * Two types of command/response channel.
> + * Type Global Command has one command/response channel.
> + * Type Stream Command has one command/response channel.
> + *----------- ------------
> + *| | --------------------------- | |
> + *| | ---->| Global Command |----> | |
> + *| | --------------------------- | |
> + *| | | |
> + *| | | |
> + *| | --------------------------- | |
> + *| | ---->| Stream Command |----> | |
> + *| | --------------------------- | |
> + *| | | |
> + *| | | |
> + *| | | |
> + *| HOST | | Firmware |
> + *| | | |
> + *| | | |
> + *| | -------------------------- | |
> + *| | <----| Global Response |<---- | |
> + *| | -------------------------- | |
> + *| | | |
> + *| | | |
> + *| | -------------------------- | |
> + *| | <----| Stream Response |<---- | |
> + *| | -------------------------- | |
> + *| | | |
> + *| | | |
> + *----------- ------------
> + */
> +
> +/*
> + * @brief command ID format
> + * cmd_id is in the format of following type:
> + * type: indicate command type, global/stream commands.
> + * group: indicate the command group.
> + * id: A unique command identification in one type and group.
> + * |<-Bit31 ~ Bit24->|<-Bit23 ~ Bit16->|<-Bit15 ~ Bit0->|
> + * | type | group | id |
> + */
> +
> +#define CMD_TYPE_SHIFT 24
> +#define CMD_GROUP_SHIFT 16
> +#define CMD_TYPE_STREAM_CTRL (0x2U << CMD_TYPE_SHIFT)
> +
> +#define CMD_GROUP_STREAM_CTRL (0x1U << CMD_GROUP_SHIFT)
> +#define CMD_GROUP_STREAM_BUFFER (0x4U << CMD_GROUP_SHIFT)
> +
> +/* Stream Command */
> +#define CMD_ID_SET_STREAM_CONFIG (CMD_TYPE_STREAM_CTRL | CMD_GROUP_STREAM_CTRL | 0x1)
> +#define CMD_ID_SET_OUT_CHAN_PROP (CMD_TYPE_STREAM_CTRL | CMD_GROUP_STREAM_CTRL | 0x3)
> +#define CMD_ID_ENABLE_OUT_CHAN (CMD_TYPE_STREAM_CTRL | CMD_GROUP_STREAM_CTRL | 0x5)
> +#define CMD_ID_START_STREAM (CMD_TYPE_STREAM_CTRL | CMD_GROUP_STREAM_CTRL | 0x7)
> +#define CMD_ID_STOP_STREAM (CMD_TYPE_STREAM_CTRL | CMD_GROUP_STREAM_CTRL | 0x8)
> +
> +/* Stream Buffer Command */
> +#define CMD_ID_SEND_BUFFER (CMD_TYPE_STREAM_CTRL | CMD_GROUP_STREAM_BUFFER | 0x1)
> +
> +/*
> + * @brief response ID format
> + * resp_id is in the format of following type:
> + * type: indicate command type, global/stream commands.
> + * group: indicate the command group.
> + * id: A unique command identification in one type and group.
> + * |<-Bit31 ~ Bit24->|<-Bit23 ~ Bit16->|<-Bit15 ~ Bit0->|
> + * | type | group | id |
> + */
> +
> +#define RESP_GROUP_SHIFT 16
> +#define RESP_GROUP_MASK (0xff << RESP_GROUP_SHIFT)
> +
> +#define GET_RESP_GROUP_VALUE(resp_id) (((resp_id) & RESP_GROUP_MASK) >> RESP_GROUP_SHIFT)
> +#define GET_RESP_ID_VALUE(resp_id) ((resp_id) & 0xffff)
> +
> +#define RESP_GROUP_GENERAL (0x1 << RESP_GROUP_SHIFT)
> +#define RESP_GROUP_NOTIFICATION (0x3 << RESP_GROUP_SHIFT)
> +
> +/* General Response */
> +#define RESP_ID_CMD_DONE (RESP_GROUP_GENERAL | 0x1)
> +
> +/* Notification */
> +#define RESP_ID_NOTI_FRAME_DONE (RESP_GROUP_NOTIFICATION | 0x1)
> +
> +#define CMD_STATUS_SUCCESS 0
> +#define CMD_STATUS_FAIL 1
> +#define CMD_STATUS_SKIPPED 2
> +
> +#define ADDR_SPACE_TYPE_GPU_VA 4
> +
> +#define FW_MEMORY_POOL_SIZE (100 * 1024 * 1024)
> +
> +/*
> + * standard ISP mipicsi=>isp
> + */
> +#define MIPI0_ISP_PIPELINE_ID 0x5f91
> +
> +enum isp4fw_sensor_id {
> + SENSOR_ID_ON_MIPI0 = 0, /* Sensor id for ISP input from MIPI port 0 */
> +};
> +
> +enum isp4fw_stream_id {
> + STREAM_ID_INVALID = -1, /* STREAM_ID_INVALID. */
> + STREAM_ID_1 = 0, /* STREAM_ID_1. */
> + STREAM_ID_2 = 1, /* STREAM_ID_2. */
> + STREAM_ID_3 = 2, /* STREAM_ID_3. */
> + STREAM_ID_MAXIMUM /* STREAM_ID_MAXIMUM. */
> +};
> +
> +enum isp4fw_image_format {
> + IMAGE_FORMAT_NV12 = 1, /* 4:2:0,semi-planar, 8-bit */
> + IMAGE_FORMAT_YUV422INTERLEAVED = 7, /* interleave, 4:2:2, 8-bit */
> +};
> +
> +enum isp4fw_pipe_out_ch {
> + ISP_PIPE_OUT_CH_PREVIEW = 0,
> +};
> +
> +enum isp4fw_yuv_range {
> + ISP_YUV_RANGE_FULL = 0, /* YUV value range in 0~255 */
> + ISP_YUV_RANGE_NARROW = 1, /* YUV value range in 16~235 */
> + ISP_YUV_RANGE_MAX
> +};
> +
> +enum isp4fw_buffer_type {
> + BUFFER_TYPE_PREVIEW = 8,
> + BUFFER_TYPE_META_INFO = 10,
> + BUFFER_TYPE_MEM_POOL = 15,
> +};
> +
> +enum isp4fw_buffer_status {
> + BUFFER_STATUS_INVALID, /* The buffer is INVALID */
> + BUFFER_STATUS_SKIPPED, /* The buffer is not filled with image data */
> + BUFFER_STATUS_EXIST, /* The buffer is exist and waiting for filled */
> + BUFFER_STATUS_DONE, /* The buffer is filled with image data */
> + BUFFER_STATUS_LACK, /* The buffer is unavailable */
> + BUFFER_STATUS_DIRTY, /* The buffer is dirty, probably caused by
> + * LMI leakage
> + */
> + BUFFER_STATUS_MAX /* The buffer STATUS_MAX */
> +};
> +
> +enum isp4fw_buffer_source {
> + /* The buffer is from the stream buffer queue */
> + BUFFER_SOURCE_STREAM,
> +};
Could you also use the ISP4 (or ISP4IF) prefix for these, please? Many look
rather generic.
...
> diff --git a/drivers/media/platform/amd/isp4/isp4_interface.h b/drivers/media/platform/amd/isp4/isp4_interface.h
> new file mode 100644
> index 000000000000..01d5268f7d4c
> --- /dev/null
> +++ b/drivers/media/platform/amd/isp4/isp4_interface.h
> @@ -0,0 +1,141 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _ISP4_INTERFACE_H_
> +#define _ISP4_INTERFACE_H_
> +
> +#include <drm/amd/isp.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +struct isp4fw_resp;
> +
> +#define ISP4IF_RB_MAX 25
> +#define ISP4IF_RESP_CHAN_TO_RB_OFFSET 9
> +#define ISP4IF_RB_PMBMAP_MEM_SIZE (SZ_16M - 1)
> +#define ISP4IF_RB_PMBMAP_MEM_CHUNK \
> + (ISP4IF_RB_PMBMAP_MEM_SIZE / (ISP4IF_RB_MAX - 1))
> +#define ISP4IF_HOST2FW_COMMAND_SIZE sizeof(struct isp4fw_cmd)
> +#define ISP4IF_MAX_NUM_HOST2FW_COMMAND 40
> +#define ISP4IF_FW_CMD_BUF_SIZE \
> + (ISP4IF_MAX_NUM_HOST2FW_COMMAND * ISP4IF_HOST2FW_COMMAND_SIZE)
> +#define ISP4IF_RB_FULL_SLEEP_US (33 * USEC_PER_MSEC)
> +#define ISP4IF_RB_FULL_TIMEOUT_US (10 * ISP4IF_RB_FULL_SLEEP_US)
> +
> +#define ISP4IF_META_INFO_BUF_SIZE ALIGN(sizeof(struct isp4fw_meta_info), 0x8000)
> +#define ISP4IF_MAX_STREAM_BUF_COUNT 8
> +
> +#define ISP4IF_FW_LOG_RINGBUF_SIZE SZ_2M
> +
> +enum isp4if_stream_id {
> + ISP4IF_STREAM_ID_GLOBAL = 0,
> + ISP4IF_STREAM_ID_1 = 1,
> + ISP4IF_STREAM_ID_MAX = 4
> +};
> +
> +enum isp4if_status {
> + ISP4IF_STATUS_PWR_OFF,
> + ISP4IF_STATUS_PWR_ON,
> + ISP4IF_STATUS_FW_RUNNING,
> + ISP4IF_FSM_STATUS_MAX
> +};
> +
> +struct isp4if_gpu_mem_info {
> + u64 mem_size;
> + u64 gpu_mc_addr;
> + void *sys_addr;
> + void *mem_handle;
> +};
> +
> +struct isp4if_img_buf_info {
> + struct {
> + void *sys_addr;
> + u64 mc_addr;
> + u32 len;
> + } planes[3];
> +};
> +
> +struct isp4if_img_buf_node {
> + struct list_head node;
> + struct isp4if_img_buf_info buf_info;
> +};
> +
> +struct isp4if_cmd_element {
> + struct list_head list;
> + u32 seq_num;
> + u32 cmd_id;
> + struct completion cmd_done;
> + atomic_t refcnt;
> +};
> +
> +struct isp4_interface {
How about just "isp4if"? Up to you.
> + struct device *dev;
> + void __iomem *mmio;
> +
> + spinlock_t cmdq_lock; /* used for cmdq access */
> + spinlock_t bufq_lock; /* used for bufq access */
> + struct mutex isp4if_mutex; /* used to send fw cmd and read fw log */
> +
> + struct list_head cmdq; /* commands sent to fw */
> + struct list_head bufq; /* buffers sent to fw */
> +
> + enum isp4if_status status;
> + u32 host2fw_seq_num;
> +
> + /* ISP fw buffers */
> + struct isp4if_gpu_mem_info *fw_log_buf;
> + struct isp4if_gpu_mem_info *fw_cmd_resp_buf;
> + struct isp4if_gpu_mem_info *fw_mem_pool;
> + struct isp4if_gpu_mem_info *meta_info_buf[ISP4IF_MAX_STREAM_BUF_COUNT];
> +};
> +
> +static inline void isp4if_split_addr64(u64 addr, u32 *lo, u32 *hi)
> +{
> + if (lo)
> + *lo = addr & 0xffffffff;
> +
> + if (hi)
> + *hi = addr >> 32;
> +}
> +
> +static inline u64 isp4if_join_addr64(u32 lo, u32 hi)
> +{
> + return (((u64)hi) << 32) | (u64)lo;
> +}
> +
> +int isp4if_f2h_resp(struct isp4_interface *ispif, enum isp4if_stream_id stream,
> + struct isp4fw_resp *resp);
> +
> +int isp4if_send_command(struct isp4_interface *ispif, u32 cmd_id, const void *package,
> + u32 package_size);
> +
> +int isp4if_send_command_sync(struct isp4_interface *ispif, u32 cmd_id, const void *package,
> + u32 package_size);
> +
> +struct isp4if_cmd_element *isp4if_rm_cmd_from_cmdq(struct isp4_interface *ispif, u32 seq_num,
> + u32 cmd_id);
> +
> +void isp4if_clear_cmdq(struct isp4_interface *ispif);
> +
> +void isp4if_clear_bufq(struct isp4_interface *ispif);
> +
> +void isp4if_dealloc_buffer_node(struct isp4if_img_buf_node *buf_node);
> +
> +struct isp4if_img_buf_node *isp4if_alloc_buffer_node(struct isp4if_img_buf_info *buf_info);
> +
> +struct isp4if_img_buf_node *isp4if_dequeue_buffer(struct isp4_interface *ispif);
> +
> +int isp4if_queue_buffer(struct isp4_interface *ispif, struct isp4if_img_buf_node *buf_node);
> +
> +int isp4if_stop(struct isp4_interface *ispif);
> +
> +int isp4if_start(struct isp4_interface *ispif);
> +
> +int isp4if_deinit(struct isp4_interface *ispif);
> +
> +int isp4if_init(struct isp4_interface *ispif, struct device *dev, void __iomem *isp_mmio);
Could you run
$ ./scripts/checkpatch.pl --strict --max-line-length=80
on the set, please?
> +
> +#endif /* _ISP4_INTERFACE_H_ */
--
Kind regards,
Sakari Ailus
Powered by blists - more mailing lists