Re: [PATCH v2 2/8] media: platform: amd: low level support for isp4 firmware

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Bin,

On Wed, Jun 18, 2025 at 05:19:53PM +0800, Bin Du wrote:
> Low level functions for access the registers and mapping to their ranges.
> This change also includes register definitions for ring buffer used to
> communicate with ISP Firmware.
> Ring buffer is the communication interface between driver and ISP Firmware.
> Command and responses are exchanged through the ring buffer.

Please rewrap this, the third line could well be longer.

> 
> Signed-off-by: Bin Du <Bin.Du@amd.com>
> Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
> ---
>  drivers/media/platform/amd/isp4/Makefile      |   3 +-
>  drivers/media/platform/amd/isp4/isp4_hw.c     |  46 +++++++
>  drivers/media/platform/amd/isp4/isp4_hw.h     |  14 +++
>  drivers/media/platform/amd/isp4/isp4_hw_reg.h | 116 ++++++++++++++++++
>  4 files changed, 178 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/platform/amd/isp4/isp4_hw.c
>  create mode 100644 drivers/media/platform/amd/isp4/isp4_hw.h
>  create mode 100644 drivers/media/platform/amd/isp4/isp4_hw_reg.h
> 
> diff --git a/drivers/media/platform/amd/isp4/Makefile b/drivers/media/platform/amd/isp4/Makefile
> index e9e84160517d..8ca1c4dfe246 100644
> --- a/drivers/media/platform/amd/isp4/Makefile
> +++ b/drivers/media/platform/amd/isp4/Makefile
> @@ -3,7 +3,8 @@
>  # 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_hw.o	\
>  
>  ccflags-y += -I$(srctree)/drivers/media/platform/amd/isp4
>  ccflags-y += -I$(srctree)/include
> diff --git a/drivers/media/platform/amd/isp4/isp4_hw.c b/drivers/media/platform/amd/isp4/isp4_hw.c
> new file mode 100644
> index 000000000000..e5315330a514
> --- /dev/null
> +++ b/drivers/media/platform/amd/isp4/isp4_hw.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/types.h>
> +
> +#include "isp4_hw.h"
> +#include "isp4_hw_reg.h"
> +
> +#define RMMIO_SIZE 524288
> +
> +u32 isp4hw_rreg(void __iomem *base, u32 reg)
> +{
> +	void __iomem *reg_addr;
> +
> +	if (reg >= RMMIO_SIZE)
> +		return RREG_FAILED_VAL;
> +
> +	if (reg < ISP_MIPI_PHY0_REG0)
> +		reg_addr = base + reg;
> +	else if (reg <= ISP_MIPI_PHY0_REG0 + ISP_MIPI_PHY0_SIZE)
> +		reg_addr = base + (reg - ISP_MIPI_PHY0_REG0);

Redundant parentheses.

> +	else
> +		return RREG_FAILED_VAL;
> +
> +	return readl(reg_addr);
> +};
> +
> +void isp4hw_wreg(void __iomem *base, u32 reg, u32 val)
> +{
> +	void __iomem *reg_addr;
> +
> +	if (reg >= RMMIO_SIZE)
> +		return;
> +
> +	if (reg < ISP_MIPI_PHY0_REG0)
> +		reg_addr = base + reg;
> +	else if (reg <= ISP_MIPI_PHY0_REG0 + ISP_MIPI_PHY0_SIZE)
> +		reg_addr = base + (reg - ISP_MIPI_PHY0_REG0);

Ditto.

> +	else
> +		return;
> +
> +	writel(val, reg_addr);
> +};
> diff --git a/drivers/media/platform/amd/isp4/isp4_hw.h b/drivers/media/platform/amd/isp4/isp4_hw.h
> new file mode 100644
> index 000000000000..072d135b9e3a
> --- /dev/null
> +++ b/drivers/media/platform/amd/isp4/isp4_hw.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _ISP4_HW_H_
> +#define _ISP4_HW_H_
> +
> +#define RREG_FAILED_VAL 0xFFFFFFFF
> +
> +u32 isp4hw_rreg(void __iomem *base, u32 reg);
> +void isp4hw_wreg(void __iomem *base, u32 reg, u32 val);
> +
> +#endif
> diff --git a/drivers/media/platform/amd/isp4/isp4_hw_reg.h b/drivers/media/platform/amd/isp4/isp4_hw_reg.h
> new file mode 100644
> index 000000000000..b11f12ba6c56
> --- /dev/null
> +++ b/drivers/media/platform/amd/isp4/isp4_hw_reg.h
> @@ -0,0 +1,116 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _ISP4_HW_REG_H_
> +#define _ISP4_HW_REG_H_
> +
> +#define ISP_SOFT_RESET		0x62000
> +#define ISP_SYS_INT0_EN		0x62010
> +#define ISP_SYS_INT0_STATUS	0x62014
> +#define ISP_SYS_INT0_ACK	0x62018
> +#define ISP_CCPU_CNTL		0x62054
> +#define ISP_STATUS		0x62058
> +#define ISP_LOG_RB_BASE_LO0	0x62148
> +#define ISP_LOG_RB_BASE_HI0	0x6214C

Lower case hexadecimals, please.

> +#define ISP_LOG_RB_SIZE0	0x62150
> +#define ISP_LOG_RB_RPTR0	0x62154
> +#define ISP_LOG_RB_WPTR0	0x62158
> +#define ISP_RB_BASE_LO1		0x62170
> +#define ISP_RB_BASE_HI1		0x62174
> +#define ISP_RB_SIZE1		0x62178
> +#define ISP_RB_RPTR1		0x6217C
> +#define ISP_RB_WPTR1		0x62180
> +#define ISP_RB_BASE_LO2		0x62184
> +#define ISP_RB_BASE_HI2		0x62188
> +#define ISP_RB_SIZE2		0x6218C
> +#define ISP_RB_RPTR2		0x62190
> +#define ISP_RB_WPTR2		0x62194
> +#define ISP_RB_BASE_LO3		0x62198
> +#define ISP_RB_BASE_HI3		0x6219C
> +#define ISP_RB_SIZE3		0x621A0
> +#define ISP_RB_RPTR3		0x621A4
> +#define ISP_RB_WPTR3		0x621A8
> +#define ISP_RB_BASE_LO4		0x621AC
> +#define ISP_RB_BASE_HI4		0x621B0
> +#define ISP_RB_SIZE4		0x621B4
> +#define ISP_RB_RPTR4		0x621B8
> +#define ISP_RB_WPTR4		0x621BC
> +#define ISP_RB_BASE_LO5		0x621C0
> +#define ISP_RB_BASE_HI5		0x621C4
> +#define ISP_RB_SIZE5		0x621C8
> +#define ISP_RB_RPTR5		0x621CC
> +#define ISP_RB_WPTR5		0x621D0
> +#define ISP_RB_BASE_LO6		0x621D4
> +#define ISP_RB_BASE_HI6		0x621D8
> +#define ISP_RB_SIZE6		0x621DC
> +#define ISP_RB_RPTR6		0x621E0
> +#define ISP_RB_WPTR6		0x621E4
> +#define ISP_RB_BASE_LO7		0x621E8
> +#define ISP_RB_BASE_HI7		0x621EC
> +#define ISP_RB_SIZE7		0x621F0
> +#define ISP_RB_RPTR7		0x621F4
> +#define ISP_RB_WPTR7		0x621F8
> +#define ISP_RB_BASE_LO8		0x621FC
> +#define ISP_RB_BASE_HI8		0x62200
> +#define ISP_RB_SIZE8		0x62204
> +#define ISP_RB_RPTR8		0x62208
> +#define ISP_RB_WPTR8		0x6220C
> +#define ISP_RB_BASE_LO9		0x62210
> +#define ISP_RB_BASE_HI9		0x62214
> +#define ISP_RB_SIZE9		0x62218
> +#define ISP_RB_RPTR9		0x6221C
> +#define ISP_RB_WPTR9		0x62220
> +#define ISP_RB_BASE_LO10	0x62224
> +#define ISP_RB_BASE_HI10	0x62228
> +#define ISP_RB_SIZE10		0x6222C
> +#define ISP_RB_RPTR10		0x62230
> +#define ISP_RB_WPTR10		0x62234
> +#define ISP_RB_BASE_LO11	0x62238
> +#define ISP_RB_BASE_HI11	0x6223C
> +#define ISP_RB_SIZE11		0x62240
> +#define ISP_RB_RPTR11		0x62244
> +#define ISP_RB_WPTR11		0x62248
> +#define ISP_RB_BASE_LO12	0x6224C
> +#define ISP_RB_BASE_HI12	0x62250
> +#define ISP_RB_SIZE12		0x62254
> +#define ISP_RB_RPTR12		0x62258
> +#define ISP_RB_WPTR12		0x6225C
> +
> +#define ISP_POWER_STATUS	0x60000
> +
> +#define ISP_MIPI_PHY0_REG0	0x66700
> +#define ISP_MIPI_PHY1_REG0	0x66780
> +#define ISP_MIPI_PHY2_REG0	0x67400
> +
> +#define ISP_MIPI_PHY0_SIZE	0xD30
> +
> +/* ISP_SOFT_RESET */
> +#define ISP_SOFT_RESET__CCPU_SOFT_RESET_MASK			0x00000001UL
> +
> +/* ISP_CCPU_CNTL */
> +#define ISP_CCPU_CNTL__CCPU_HOST_SOFT_RST_MASK			0x00040000UL
> +
> +/* ISP_STATUS */
> +#define ISP_STATUS__CCPU_REPORT_MASK				0x000000feUL
> +
> +/* ISP_SYS_INT0_STATUS */
> +#define ISP_SYS_INT0_STATUS__SYS_INT_RINGBUFFER_WPT9_INT_MASK	0x00010000UL
> +#define ISP_SYS_INT0_STATUS__SYS_INT_RINGBUFFER_WPT10_INT_MASK	0x00040000UL
> +#define ISP_SYS_INT0_STATUS__SYS_INT_RINGBUFFER_WPT11_INT_MASK	0x00100000UL
> +#define ISP_SYS_INT0_STATUS__SYS_INT_RINGBUFFER_WPT12_INT_MASK	0x00400000UL
> +
> +/* ISP_SYS_INT0_EN */
> +#define ISP_SYS_INT0_EN__SYS_INT_RINGBUFFER_WPT9_EN_MASK	0x00010000UL
> +#define ISP_SYS_INT0_EN__SYS_INT_RINGBUFFER_WPT10_EN_MASK	0x00040000UL
> +#define ISP_SYS_INT0_EN__SYS_INT_RINGBUFFER_WPT11_EN_MASK	0x00100000UL
> +#define ISP_SYS_INT0_EN__SYS_INT_RINGBUFFER_WPT12_EN_MASK	0x00400000UL
> +
> +/* ISP_SYS_INT0_ACK */
> +#define ISP_SYS_INT0_ACK__SYS_INT_RINGBUFFER_WPT9_ACK_MASK	0x00010000UL
> +#define ISP_SYS_INT0_ACK__SYS_INT_RINGBUFFER_WPT10_ACK_MASK	0x00040000UL
> +#define ISP_SYS_INT0_ACK__SYS_INT_RINGBUFFER_WPT11_ACK_MASK	0x00100000UL
> +#define ISP_SYS_INT0_ACK__SYS_INT_RINGBUFFER_WPT12_ACK_MASK	0x00400000UL
> +
> +#endif

-- 
Kind regards,

Sakari Ailus