[PATCH v7 0/5] media: renesas: vsp1: Add support for VSPX and IIF

[PATCH v7 0/5] media: renesas: vsp1: Add support for VSPX and IIF

[Jacopo Mondi]

The VSPX is a VSP2 function that reads data from
external memory using two RPF instances and feed it to the ISP.

The VSPX includes an IIF unit (ISP InterFace) modeled in the vsp1 driver
as a new, simple, entity type.

IIF is part of VSPX, a version of the VSP2 IP specialized for ISP
interfacing. To prepare to support VSPX, support IIF first by
introducing a new entity and by adjusting the RPF/WPF drivers to
operate correctly when an IIF is present.

Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
Changes in v7:
- Include VSPX driver in the series
- Use existing VSP1 formats and remove patches extending formats on RPF
- Rework VSPX driver to split jobs creation and scheduling in two
  different API entry points
- Fix VSPX stride using the user provided bytesperline and using the
  buffer size for ConfigDMA buffers
- Link to v6: https://lore.kernel.org/r/20250321-v4h-iif-v6-0-361e9043026a@ideasonboard.com

Changes in v6:
- Little cosmetic change as suggested by Laurent
- Collect tags
- Link to v5: https://lore.kernel.org/r/20250319-v4h-iif-v5-0-0a10456d792c@ideasonboard.com

Changes in v5:
- Drop additional empty line 5/6
- Link to v4: https://lore.kernel.org/r/20250318-v4h-iif-v4-0-10ed4c41c195@ideasonboard.com

Changes in v4:
- Fix SWAP bits for RAW10, RAW12 and RAW16
- Link to v3: https://lore.kernel.org/r/20250317-v4h-iif-v3-0-63aab8982b50@ideasonboard.com

Changes in v3:
- Drop 2/6 from v2
- Add 5/7 to prepare for a new implementation of 6/7
- Individual changelog per patch
- Add 7/7
- Link to v2: https://lore.kernel.org/r/20250224-v4h-iif-v2-0-0305e3c1fe2d@ideasonboard.com

Changes in v2:
- Collect tags
- Address review comments from Laurent, a lot of tiny changes here and
  there but no major redesign worth an entry in the patchset changelog

---
Jacopo Mondi (5):
      media: vsp1: Add support IIF ISP Interface
      media: vsp1: dl: Use singleshot DL for VSPX
      media: vsp1: wpf: Propagate vsp1_rwpf_init_ctrls()
      media: vsp1: rwpf: Support operations with IIF
      media: vsp1: Add VSPX support

 drivers/media/platform/renesas/vsp1/Makefile      |   3 +-
 drivers/media/platform/renesas/vsp1/vsp1.h        |   4 +
 drivers/media/platform/renesas/vsp1/vsp1_dl.c     |   7 +-
 drivers/media/platform/renesas/vsp1/vsp1_drv.c    |  24 +-
 drivers/media/platform/renesas/vsp1/vsp1_entity.c |   8 +
 drivers/media/platform/renesas/vsp1/vsp1_entity.h |   1 +
 drivers/media/platform/renesas/vsp1/vsp1_iif.c    | 121 +++++
 drivers/media/platform/renesas/vsp1/vsp1_iif.h    |  29 ++
 drivers/media/platform/renesas/vsp1/vsp1_pipe.c   |   1 +
 drivers/media/platform/renesas/vsp1/vsp1_pipe.h   |   1 +
 drivers/media/platform/renesas/vsp1/vsp1_regs.h   |   9 +
 drivers/media/platform/renesas/vsp1/vsp1_rpf.c    |   9 +-
 drivers/media/platform/renesas/vsp1/vsp1_vspx.c   | 604 ++++++++++++++++++++++
 drivers/media/platform/renesas/vsp1/vsp1_vspx.h   |  86 +++
 drivers/media/platform/renesas/vsp1/vsp1_wpf.c    |  24 +-
 include/media/vsp1.h                              |  73 +++
 16 files changed, 991 insertions(+), 13 deletions(-)
---
base-commit: f2151613e040973c868d28c8b00885dfab69eb75
change-id: 20250123-v4h-iif-a1dda640c95d

Best regards,
-- 
Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

[PATCH v7 1/5] media: vsp1: Add support IIF ISP Interface

[Jacopo Mondi]

The IIF (ISP InterFace) is a VSP2 function that transfers data
to the ISP by reading from external memory through two RPF
instances.

Add support for it in the vsp1 driver by introducing a new entity
type. The sole required operation is to enable the IIF function
during configure_stream().

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

---
v2->v3:
- Replace SRGGB codes with luma-only
- Fix CI loop warning report
- Drop a hunk that was not meant to be there
---
 drivers/media/platform/renesas/vsp1/Makefile      |   2 +-
 drivers/media/platform/renesas/vsp1/vsp1.h        |   3 +
 drivers/media/platform/renesas/vsp1/vsp1_drv.c    |  11 ++
 drivers/media/platform/renesas/vsp1/vsp1_entity.c |   8 ++
 drivers/media/platform/renesas/vsp1/vsp1_entity.h |   1 +
 drivers/media/platform/renesas/vsp1/vsp1_iif.c    | 121 ++++++++++++++++++++++
 drivers/media/platform/renesas/vsp1/vsp1_iif.h    |  26 +++++
 drivers/media/platform/renesas/vsp1/vsp1_pipe.c   |   1 +
 drivers/media/platform/renesas/vsp1/vsp1_pipe.h   |   1 +
 drivers/media/platform/renesas/vsp1/vsp1_regs.h   |   8 ++
 10 files changed, 181 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/vsp1/Makefile b/drivers/media/platform/renesas/vsp1/Makefile
index 4bb4dcbef7b5..de8c802e1d1a 100644
--- a/drivers/media/platform/renesas/vsp1/Makefile
+++ b/drivers/media/platform/renesas/vsp1/Makefile
@@ -5,6 +5,6 @@ vsp1-y					+= vsp1_rpf.o vsp1_rwpf.o vsp1_wpf.o
 vsp1-y					+= vsp1_clu.o vsp1_hsit.o vsp1_lut.o
 vsp1-y					+= vsp1_brx.o vsp1_sru.o vsp1_uds.o
 vsp1-y					+= vsp1_hgo.o vsp1_hgt.o vsp1_histo.o
-vsp1-y					+= vsp1_lif.o vsp1_uif.o
+vsp1-y					+= vsp1_iif.o vsp1_lif.o vsp1_uif.o
 
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1.o
diff --git a/drivers/media/platform/renesas/vsp1/vsp1.h b/drivers/media/platform/renesas/vsp1/vsp1.h
index 2f6f0c6ae555..263024639dd2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1.h
@@ -32,6 +32,7 @@ struct vsp1_clu;
 struct vsp1_hgo;
 struct vsp1_hgt;
 struct vsp1_hsit;
+struct vsp1_iif;
 struct vsp1_lif;
 struct vsp1_lut;
 struct vsp1_rwpf;
@@ -56,6 +57,7 @@ struct vsp1_uif;
 #define VSP1_HAS_BRS		BIT(9)
 #define VSP1_HAS_EXT_DL		BIT(10)
 #define VSP1_HAS_NON_ZERO_LBA	BIT(11)
+#define VSP1_HAS_IIF		BIT(12)
 
 struct vsp1_device_info {
 	u32 version;
@@ -91,6 +93,7 @@ struct vsp1_device {
 	struct vsp1_hgt *hgt;
 	struct vsp1_hsit *hsi;
 	struct vsp1_hsit *hst;
+	struct vsp1_iif *iif;
 	struct vsp1_lif *lif[VSP1_MAX_LIF];
 	struct vsp1_lut *lut;
 	struct vsp1_rwpf *rpf[VSP1_MAX_RPF];
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
index 9fc6bf624a52..d13e9b31aa7c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
@@ -29,6 +29,7 @@
 #include "vsp1_hgo.h"
 #include "vsp1_hgt.h"
 #include "vsp1_hsit.h"
+#include "vsp1_iif.h"
 #include "vsp1_lif.h"
 #include "vsp1_lut.h"
 #include "vsp1_pipe.h"
@@ -340,6 +341,16 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 			      &vsp1->entities);
 	}
 
+	if (vsp1_feature(vsp1, VSP1_HAS_IIF)) {
+		vsp1->iif = vsp1_iif_create(vsp1);
+		if (IS_ERR(vsp1->iif)) {
+			ret = PTR_ERR(vsp1->iif);
+			goto done;
+		}
+
+		list_add_tail(&vsp1->iif->entity.list_dev, &vsp1->entities);
+	}
+
 	/*
 	 * The LIFs are only supported when used in conjunction with the DU, in
 	 * which case the userspace API is disabled. If the userspace API is
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 8b8945bd8f10..2096a09a1278 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -63,9 +63,14 @@ void vsp1_entity_route_setup(struct vsp1_entity *entity,
 	/*
 	 * The ILV and BRS share the same data path route. The extra BRSSEL bit
 	 * selects between the ILV and BRS.
+	 *
+	 * The BRU and IIF share the same data path route. The extra IIFSEL bit
+	 * selects between the IIF and BRU.
 	 */
 	if (source->type == VSP1_ENTITY_BRS)
 		route |= VI6_DPR_ROUTE_BRSSEL;
+	else if (source->type == VSP1_ENTITY_IIF)
+		route |= VI6_DPR_ROUTE_IIFSEL;
 	vsp1_dl_body_write(dlb, source->route->reg, route);
 }
 
@@ -528,6 +533,9 @@ struct media_pad *vsp1_entity_remote_pad(struct media_pad *pad)
 	  { VI6_DPR_NODE_WPF(idx) }, VI6_DPR_NODE_WPF(idx) }
 
 static const struct vsp1_route vsp1_routes[] = {
+	{ VSP1_ENTITY_IIF, 0, VI6_DPR_BRU_ROUTE,
+	  { VI6_DPR_NODE_BRU_IN(0), VI6_DPR_NODE_BRU_IN(1),
+	    VI6_DPR_NODE_BRU_IN(3) }, VI6_DPR_NODE_WPF(0) },
 	{ VSP1_ENTITY_BRS, 0, VI6_DPR_ILV_BRS_ROUTE,
 	  { VI6_DPR_NODE_BRS_IN(0), VI6_DPR_NODE_BRS_IN(1) }, 0 },
 	{ VSP1_ENTITY_BRU, 0, VI6_DPR_BRU_ROUTE,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index 1bcc9e27dfdc..bdcb780a79da 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -28,6 +28,7 @@ enum vsp1_entity_type {
 	VSP1_ENTITY_HGT,
 	VSP1_ENTITY_HSI,
 	VSP1_ENTITY_HST,
+	VSP1_ENTITY_IIF,
 	VSP1_ENTITY_LIF,
 	VSP1_ENTITY_LUT,
 	VSP1_ENTITY_RPF,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_iif.c b/drivers/media/platform/renesas/vsp1/vsp1_iif.c
new file mode 100644
index 000000000000..5dd62bebbe8c
--- /dev/null
+++ b/drivers/media/platform/renesas/vsp1/vsp1_iif.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * vsp1_iif.c  --  R-Car VSP1 IIF (ISP Interface)
+ *
+ * Copyright (C) 2025 Ideas On Board Oy
+ * Copyright (C) 2025 Renesas Corporation
+ */
+
+#include "vsp1.h"
+#include "vsp1_dl.h"
+#include "vsp1_iif.h"
+
+#define IIF_MIN_WIDTH				128U
+#define IIF_MIN_HEIGHT				32U
+#define IIF_MAX_WIDTH				5120U
+#define IIF_MAX_HEIGHT				4096U
+
+/* -----------------------------------------------------------------------------
+ * Device Access
+ */
+
+static inline void vsp1_iif_write(struct vsp1_dl_body *dlb, u32 reg, u32 data)
+{
+	vsp1_dl_body_write(dlb, reg, data);
+}
+
+/* -----------------------------------------------------------------------------
+ * V4L2 Subdevice Operations
+ */
+
+static const unsigned int iif_codes[] = {
+	MEDIA_BUS_FMT_Y8_1X8,
+	MEDIA_BUS_FMT_Y10_1X10,
+	MEDIA_BUS_FMT_Y12_1X12,
+	MEDIA_BUS_FMT_Y16_1X16,
+	MEDIA_BUS_FMT_METADATA_FIXED
+};
+
+static int iif_enum_mbus_code(struct v4l2_subdev *subdev,
+			      struct v4l2_subdev_state *sd_state,
+			      struct v4l2_subdev_mbus_code_enum *code)
+{
+	return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, iif_codes,
+					  ARRAY_SIZE(iif_codes));
+}
+
+static int iif_enum_frame_size(struct v4l2_subdev *subdev,
+			       struct v4l2_subdev_state *sd_state,
+			       struct v4l2_subdev_frame_size_enum *fse)
+{
+	return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
+					   IIF_MIN_WIDTH, IIF_MIN_HEIGHT,
+					   IIF_MAX_WIDTH, IIF_MAX_HEIGHT);
+}
+
+static int iif_set_format(struct v4l2_subdev *subdev,
+			  struct v4l2_subdev_state *sd_state,
+			  struct v4l2_subdev_format *fmt)
+{
+	return vsp1_subdev_set_pad_format(subdev, sd_state, fmt, iif_codes,
+					  ARRAY_SIZE(iif_codes),
+					  IIF_MIN_WIDTH, IIF_MIN_HEIGHT,
+					  IIF_MAX_WIDTH, IIF_MAX_HEIGHT);
+}
+
+static const struct v4l2_subdev_pad_ops iif_pad_ops = {
+	.enum_mbus_code = iif_enum_mbus_code,
+	.enum_frame_size = iif_enum_frame_size,
+	.get_fmt = vsp1_subdev_get_pad_format,
+	.set_fmt = iif_set_format,
+};
+
+static const struct v4l2_subdev_ops iif_ops = {
+	.pad    = &iif_pad_ops,
+};
+
+/* -----------------------------------------------------------------------------
+ * VSP1 Entity Operations
+ */
+
+static void iif_configure_stream(struct vsp1_entity *entity,
+				 struct v4l2_subdev_state *state,
+				 struct vsp1_pipeline *pipe,
+				 struct vsp1_dl_list *dl,
+				 struct vsp1_dl_body *dlb)
+{
+	vsp1_iif_write(dlb, VI6_IIF_CTRL, VI6_IIF_CTRL_CTRL);
+}
+
+static const struct vsp1_entity_operations iif_entity_ops = {
+	.configure_stream = iif_configure_stream,
+};
+
+/* -----------------------------------------------------------------------------
+ * Initialization and Cleanup
+ */
+
+struct vsp1_iif *vsp1_iif_create(struct vsp1_device *vsp1)
+{
+	struct vsp1_iif *iif;
+	int ret;
+
+	iif = devm_kzalloc(vsp1->dev, sizeof(*iif), GFP_KERNEL);
+	if (!iif)
+		return ERR_PTR(-ENOMEM);
+
+	iif->entity.ops = &iif_entity_ops;
+	iif->entity.type = VSP1_ENTITY_IIF;
+
+	/*
+	 * The IIF is never exposed to userspace, but media entity registration
+	 * requires a function to be set. Use PROC_VIDEO_PIXEL_FORMATTER just to
+	 * avoid triggering a WARN_ON(), the value won't be seen anywhere.
+	 */
+	ret = vsp1_entity_init(vsp1, &iif->entity, "iif", 3, &iif_ops,
+			       MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	return iif;
+}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_iif.h b/drivers/media/platform/renesas/vsp1/vsp1_iif.h
new file mode 100644
index 000000000000..165996a822c1
--- /dev/null
+++ b/drivers/media/platform/renesas/vsp1/vsp1_iif.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * vsp1_iif.h  --  R-Car VSP1 IIF (ISP Interface)
+ *
+ * Copyright (C) 2025 Ideas On Board Oy
+ * Copyright (C) 2025 Renesas Corporation
+ */
+#ifndef __VSP1_IIF_H__
+#define __VSP1_IIF_H__
+
+#include <media/v4l2-subdev.h>
+
+#include "vsp1_entity.h"
+
+struct vsp1_iif {
+	struct vsp1_entity entity;
+};
+
+static inline struct vsp1_iif *to_iif(struct v4l2_subdev *subdev)
+{
+	return container_of(subdev, struct vsp1_iif, entity.subdev);
+}
+
+struct vsp1_iif *vsp1_iif_create(struct vsp1_device *vsp1);
+
+#endif /* __VSP1_IIF_H__ */
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
index bb0739f684f3..8e9be3ec1b4d 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
@@ -286,6 +286,7 @@ void vsp1_pipeline_reset(struct vsp1_pipeline *pipe)
 	pipe->brx = NULL;
 	pipe->hgo = NULL;
 	pipe->hgt = NULL;
+	pipe->iif = NULL;
 	pipe->lif = NULL;
 	pipe->uds = NULL;
 }
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
index 1ba7bdbad5a8..1655a820da10 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
@@ -119,6 +119,7 @@ struct vsp1_pipeline {
 	struct vsp1_entity *brx;
 	struct vsp1_entity *hgo;
 	struct vsp1_entity *hgt;
+	struct vsp1_entity *iif;
 	struct vsp1_entity *lif;
 	struct vsp1_entity *uds;
 	struct vsp1_entity *uds_input;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_regs.h b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
index 7eca82e0ba7e..86e47c2d991f 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
@@ -252,6 +252,13 @@
 #define VI6_RPF_BRDITH_CTRL_ODE		BIT(8)
 #define VI6_RPF_BRDITH_CTRL_CBRM	BIT(0)
 
+/* -----------------------------------------------------------------------------
+ * IIF Control Registers
+ */
+
+#define VI6_IIF_CTRL			0x0608
+#define VI6_IIF_CTRL_CTRL		0x13
+
 /* -----------------------------------------------------------------------------
  * WPF Control Registers
  */
@@ -388,6 +395,7 @@
 #define VI6_DPR_HST_ROUTE		0x2044
 #define VI6_DPR_HSI_ROUTE		0x2048
 #define VI6_DPR_BRU_ROUTE		0x204c
+#define VI6_DPR_ROUTE_IIFSEL		BIT(28)
 #define VI6_DPR_ILV_BRS_ROUTE		0x2050
 #define VI6_DPR_ROUTE_BRSSEL		BIT(28)
 #define VI6_DPR_ROUTE_FXA_MASK		(0xff << 16)

-- 
2.48.1

[PATCH v7 2/5] media: vsp1: dl: Use singleshot DL for VSPX

[Jacopo Mondi]

The vsp1_dl library allows to program a display list and feed it
continuously to the VSP2. As an alternative operation mode, the library
allows to program the VSP2 in 'single shot' mode, where a display list
is submitted to the VSP on request only.

Currently the 'single shot' mode is only available when the VSP2 is
controlled by userspace, while it works in continuous mode when driven
by DRM, as frames have to be submitted to the display continuously.

For the VSPX use case, where there is no uapi support, we should however
work in single-shot mode as the ISP driver programs the VSPX on
request.

Initialize the display lists in single shot mode in case the VSP1 is
controlled by userspace or in case the pipeline features an IIF entity,
as in the VSPX case.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

---
v2->v3:
 - Drop () in condition
---
 drivers/media/platform/renesas/vsp1/vsp1_dl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_dl.c b/drivers/media/platform/renesas/vsp1/vsp1_dl.c
index ad3fa1c9cc73..bb8228b19824 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_dl.c
@@ -1099,7 +1099,12 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
 		return NULL;
 
 	dlm->index = index;
-	dlm->singleshot = vsp1->info->uapi;
+	/*
+	 * uapi = single shot mode;
+	 * DRM = continuous mode;
+	 * VSPX = single shot mode;
+	 */
+	dlm->singleshot = vsp1->info->uapi || vsp1->iif;
 	dlm->vsp1 = vsp1;
 
 	spin_lock_init(&dlm->lock);

-- 
2.48.1

[PATCH v7 3/5] media: vsp1: wpf: Propagate vsp1_rwpf_init_ctrls()

[Jacopo Mondi]

vsp1_wpf.c calls vsp1_rwpf_init_ctrls() to initialize controls that
are common between RPF and WPF.

However, the vsp1_wpf.c implementation does not check for the function
call return value. Fix this by propagating to the caller the return
value.

While at it, drop a duplicated error message in wpf_init_controls() as
the caller already report it.

Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---
v2->v3:
  - New patch
---
 drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index f176750ccd98..da651a882bbb 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -133,6 +133,7 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
 {
 	struct vsp1_device *vsp1 = wpf->entity.vsp1;
 	unsigned int num_flip_ctrls;
+	int ret;
 
 	spin_lock_init(&wpf->flip.lock);
 
@@ -156,7 +157,9 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
 		num_flip_ctrls = 0;
 	}
 
-	vsp1_rwpf_init_ctrls(wpf, num_flip_ctrls);
+	ret = vsp1_rwpf_init_ctrls(wpf, num_flip_ctrls);
+	if (ret < 0)
+		return ret;
 
 	if (num_flip_ctrls >= 1) {
 		wpf->flip.ctrls.vflip =
@@ -174,11 +177,8 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
 		v4l2_ctrl_cluster(3, &wpf->flip.ctrls.vflip);
 	}
 
-	if (wpf->ctrls.error) {
-		dev_err(vsp1->dev, "wpf%u: failed to initialize controls\n",
-			wpf->entity.index);
+	if (wpf->ctrls.error)
 		return wpf->ctrls.error;
-	}
 
 	return 0;
 }

-- 
2.48.1

[PATCH v7 4/5] media: vsp1: rwpf: Support operations with IIF

[Jacopo Mondi]

When the RPF/WPF units are used for ISP interfacing through
the IIF, the set of accessible registers is limited compared to
the regular VSPD operations.

Support ISP interfacing in the rpf and wpf entities by checking if
the pipe features an IIF instance and writing only the relevant
registers.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/vsp1/vsp1_iif.h |  3 +++
 drivers/media/platform/renesas/vsp1/vsp1_rpf.c |  9 ++++++++-
 drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 14 ++++++++++----
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_iif.h b/drivers/media/platform/renesas/vsp1/vsp1_iif.h
index 165996a822c1..46f327851c35 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_iif.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_iif.h
@@ -12,6 +12,9 @@
 
 #include "vsp1_entity.h"
 
+#define VSPX_IIF_SINK_PAD_IMG		0
+#define VSPX_IIF_SINK_PAD_CONFIG	2
+
 struct vsp1_iif {
 	struct vsp1_entity entity;
 };
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 5c8b3ba1bd3c..5e84536f0cdd 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -84,7 +84,7 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
 	sink_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
 	source_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SOURCE);
 
-	infmt = VI6_RPF_INFMT_CIPM
+	infmt = (pipe->iif ? 0 : VI6_RPF_INFMT_CIPM)
 	      | (fmtinfo->hwfmt << VI6_RPF_INFMT_RDFMT_SHIFT);
 
 	if (fmtinfo->swap_yc)
@@ -98,6 +98,13 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
 	vsp1_rpf_write(rpf, dlb, VI6_RPF_INFMT, infmt);
 	vsp1_rpf_write(rpf, dlb, VI6_RPF_DSWAP, fmtinfo->swap);
 
+	/* No further configuration for VSPX. */
+	if (pipe->iif) {
+		/* VSPX wants alpha_sel to be set to 0. */
+		vsp1_rpf_write(rpf, dlb, VI6_RPF_ALPH_SEL, 0);
+		return;
+	}
+
 	if (entity->vsp1->info->gen == 4) {
 		u32 ext_infmt0;
 		u32 ext_infmt1;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index da651a882bbb..f3ea3b17e4cb 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -247,8 +247,11 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
 	sink_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
 	source_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SOURCE);
 
-	/* Format */
-	if (!pipe->lif || wpf->writeback) {
+	/*
+	 * Format configuration. Skip for IIF (VSPX) or if the pipe doesn't
+	 * write to memory.
+	 */
+	if (!pipe->iif && (!pipe->lif || wpf->writeback)) {
 		const struct v4l2_pix_format_mplane *format = &wpf->format;
 		const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
 
@@ -291,7 +294,7 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
 	 * Sources. If the pipeline has a single input and BRx is not used,
 	 * configure it as the master layer. Otherwise configure all
 	 * inputs as sub-layers and select the virtual RPF as the master
-	 * layer.
+	 * layer. For VSPX configure the enabled sources as masters.
 	 */
 	for (i = 0; i < vsp1->info->rpf_count; ++i) {
 		struct vsp1_rwpf *input = pipe->inputs[i];
@@ -299,7 +302,7 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
 		if (!input)
 			continue;
 
-		srcrpf |= (!pipe->brx && pipe->num_inputs == 1)
+		srcrpf |= (pipe->iif || (!pipe->brx && pipe->num_inputs == 1))
 			? VI6_WPF_SRCRPF_RPF_ACT_MST(input->entity.index)
 			: VI6_WPF_SRCRPF_RPF_ACT_SUB(input->entity.index);
 	}
@@ -316,6 +319,9 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
 	vsp1_dl_body_write(dlb, VI6_WPF_IRQ_ENB(index),
 			   VI6_WPF_IRQ_ENB_DFEE);
 
+	if (pipe->iif)
+		return;
+
 	/*
 	 * Configure writeback for display pipelines (the wpf writeback flag is
 	 * never set for memory-to-memory pipelines). Start by adding a chained

-- 
2.48.1

[PATCH v7 5/5] media: vsp1: Add VSPX support

[Jacopo Mondi]

Add support for VSPX, a specialized version of the VSP2 that
transfer data to the ISP. The VSPX is composed by two RPF units
to read data from external memory and an IIF instance that performs
transfer towards the ISP.

The VSPX is supported through a newly introduced vsp1_vspx.c file that
exposes two interfaces: vsp1_vspx interface, declared in vsp1_vspx.h
for the vsp1 core to initialize and cleanup the VSPX, and a vsp1_isp
interface, declared in include/media/vsp1.h for the ISP driver to
control the VSPX operations.

Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/vsp1/Makefile    |   1 +
 drivers/media/platform/renesas/vsp1/vsp1.h      |   1 +
 drivers/media/platform/renesas/vsp1/vsp1_drv.c  |  13 +-
 drivers/media/platform/renesas/vsp1/vsp1_regs.h |   1 +
 drivers/media/platform/renesas/vsp1/vsp1_vspx.c | 604 ++++++++++++++++++++++++
 drivers/media/platform/renesas/vsp1/vsp1_vspx.h |  86 ++++
 include/media/vsp1.h                            |  73 +++
 7 files changed, 778 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/vsp1/Makefile b/drivers/media/platform/renesas/vsp1/Makefile
index de8c802e1d1a..2057c8f7be47 100644
--- a/drivers/media/platform/renesas/vsp1/Makefile
+++ b/drivers/media/platform/renesas/vsp1/Makefile
@@ -6,5 +6,6 @@ vsp1-y					+= vsp1_clu.o vsp1_hsit.o vsp1_lut.o
 vsp1-y					+= vsp1_brx.o vsp1_sru.o vsp1_uds.o
 vsp1-y					+= vsp1_hgo.o vsp1_hgt.o vsp1_histo.o
 vsp1-y					+= vsp1_iif.o vsp1_lif.o vsp1_uif.o
+vsp1-y					+= vsp1_vspx.o
 
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1.o
diff --git a/drivers/media/platform/renesas/vsp1/vsp1.h b/drivers/media/platform/renesas/vsp1/vsp1.h
index 263024639dd2..1872e403f26b 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1.h
@@ -110,6 +110,7 @@ struct vsp1_device {
 	struct media_entity_operations media_ops;
 
 	struct vsp1_drm *drm;
+	struct vsp1_vspx *vspx;
 };
 
 int vsp1_device_get(struct vsp1_device *vsp1);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
index d13e9b31aa7c..e338ab8af292 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
@@ -38,6 +38,7 @@
 #include "vsp1_uds.h"
 #include "vsp1_uif.h"
 #include "vsp1_video.h"
+#include "vsp1_vspx.h"
 
 /* -----------------------------------------------------------------------------
  * Interrupt Handling
@@ -488,7 +489,10 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 
 		ret = media_device_register(mdev);
 	} else {
-		ret = vsp1_drm_init(vsp1);
+		if (vsp1->info->version == VI6_IP_VERSION_MODEL_VSPX_GEN4)
+			ret = vsp1_vspx_init(vsp1);
+		else
+			ret = vsp1_drm_init(vsp1);
 	}
 
 done:
@@ -846,6 +850,13 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
 		.uif_count = 2,
 		.wpf_count = 1,
 		.num_bru_inputs = 5,
+	}, {
+		.version = VI6_IP_VERSION_MODEL_VSPX_GEN4,
+		.model = "VSP2-X",
+		.gen = 4,
+		.features = VSP1_HAS_IIF,
+		.rpf_count = 2,
+		.wpf_count = 1,
 	},
 };
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_regs.h b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
index 86e47c2d991f..10cfbcd1b6e0 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
@@ -799,6 +799,7 @@
 #define VI6_IP_VERSION_MODEL_VSPDL_GEN3	(0x19 << 8)
 #define VI6_IP_VERSION_MODEL_VSPBS_GEN3	(0x1a << 8)
 #define VI6_IP_VERSION_MODEL_VSPD_GEN4	(0x1c << 8)
+#define VI6_IP_VERSION_MODEL_VSPX_GEN4	(0x1d << 8)
 /* RZ/G2L SoCs have no version register, So use 0x80 as the model version */
 #define VI6_IP_VERSION_MODEL_VSPD_RZG2L	(0x80 << 8)
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_vspx.c b/drivers/media/platform/renesas/vsp1/vsp1_vspx.c
new file mode 100644
index 000000000000..db9707f2b532
--- /dev/null
+++ b/drivers/media/platform/renesas/vsp1/vsp1_vspx.c
@@ -0,0 +1,604 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * vsp1_vspx.c  --  R-Car Gen 4 VSPX
+ *
+ * Copyright (C) 2025 Ideas On Board Oy
+ * Copyright (C) 2025 Renesas Electronics Corporation
+ */
+
+#include "vsp1_vspx.h"
+
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/slab.h>
+
+#include <media/media-entity.h>
+#include <media/v4l2-subdev.h>
+#include <media/vsp1.h>
+
+#include "vsp1.h"
+#include "vsp1_dl.h"
+#include "vsp1_iif.h"
+#include "vsp1_pipe.h"
+#include "vsp1_rwpf.h"
+
+static const struct v4l2_pix_format_mplane vspx_default_fmt = {
+	.width = 1920,
+	.height = 1080,
+	.pixelformat = V4L2_PIX_FMT_SRGGB8,
+	.field = V4L2_FIELD_NONE,
+	.num_planes = 1,
+	.plane_fmt = {
+		[0] = {
+			.sizeimage = 1920 * 1080,
+			.bytesperline = 1920,
+		},
+	},
+};
+
+/*
+ * Apply the given width, height and fourcc to the subdevice inside the
+ * VSP1 entity.
+ */
+static int vsp1_vspx_rwpf_set_subdev_fmt(struct vsp1_device *vsp1,
+					 struct vsp1_rwpf *rwpf,
+					 u32 isp_fourcc,
+					 unsigned int width,
+					 unsigned int height)
+{
+	struct vsp1_entity *ent = &rwpf->entity;
+	const struct vsp1_format_info *fmtinfo;
+	struct v4l2_subdev_format format = {};
+	u32 vspx_fourcc;
+	int ret;
+
+	switch (isp_fourcc) {
+	case V4L2_PIX_FMT_GREY:
+		/* 8 bit RAW Bayer */
+		vspx_fourcc = V4L2_PIX_FMT_RGB332;
+		break;
+	case V4L2_PIX_FMT_Y10:
+	case V4L2_PIX_FMT_Y12:
+	case V4L2_PIX_FMT_Y16:
+		/* 10, 12 and 16 bit RAW Bayer */
+		vspx_fourcc = V4L2_PIX_FMT_RGB565;
+		break;
+	case V4L2_PIX_FMT_XBGR32:
+		/* ConfigDMA */
+		vspx_fourcc = V4L2_PIX_FMT_XBGR32;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	fmtinfo = vsp1_get_format_info(vsp1, vspx_fourcc);
+	if (!fmtinfo) {
+		dev_dbg(vsp1->dev, "Unsupported pixel format %08x\n",
+			vspx_fourcc);
+		return -EINVAL;
+	}
+
+	rwpf->fmtinfo = fmtinfo;
+
+	format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+	format.pad = RWPF_PAD_SINK;
+	format.format.width = width;
+	format.format.height = height;
+	format.format.field = V4L2_FIELD_NONE;
+	format.format.code = fmtinfo->mbus;
+
+	ret = v4l2_subdev_call(&ent->subdev, pad, set_fmt, NULL, &format);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+/*
+ * Configure RPF0 for ConfigDMA or RAW image transfer.
+ */
+static int vsp1_vspx_rpf0_configure(struct vsp1_device *vsp1,
+				    dma_addr_t addr, u32 isp_fourcc,
+				    unsigned int width, unsigned int height,
+				    unsigned int stride,
+				    unsigned int iif_sink_pad,
+				    struct vsp1_dl_list *dl,
+				    struct vsp1_dl_body *dlb)
+{
+	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
+	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
+	struct vsp1_rwpf *rpf0 = pipe->inputs[0];
+	int ret;
+
+	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, rpf0, isp_fourcc, width,
+					    height);
+	if (ret)
+		return ret;
+
+	rpf0->format.plane_fmt[0].bytesperline = stride;
+
+	/*
+	 * Connect RPF0 to the IIF sink pad corresponding to the config or image
+	 * path.
+	 */
+	rpf0->entity.sink_pad = iif_sink_pad;
+
+	pipe->part_table[0].rpf[0].width = width;
+	pipe->part_table[0].rpf[0].height = height;
+
+	rpf0->mem.addr[0] = addr;
+	rpf0->mem.addr[1] = 0;
+	rpf0->mem.addr[2] = 0;
+
+	vsp1_entity_route_setup(&rpf0->entity, pipe, dlb);
+	vsp1_entity_configure_stream(&rpf0->entity, rpf0->entity.state, pipe,
+				     dl, dlb);
+	vsp1_entity_configure_partition(&rpf0->entity, pipe,
+					&pipe->part_table[0], dl, dlb);
+
+	return 0;
+}
+
+/* -----------------------------------------------------------------------------
+ * Interrupt handling
+ */
+static void vsp1_vspx_pipeline_frame_end(struct vsp1_pipeline *pipe,
+					 unsigned int completion)
+{
+	struct vsp1_vspx_pipeline *vspx_pipe = to_vsp1_vspx_pipeline(pipe);
+	struct vsp1_vspx *vspx = to_vsp1_vspx(vspx_pipe);
+
+	if (vspx_pipe->vspx_frame_end)
+		vspx_pipe->vspx_frame_end(vspx_pipe->frame_end_data);
+
+	/*
+	 * Set the pipeline state to stopped to ensure the next call to
+	 * vsp1_pipeline_run() actually starts the VSPX.
+	 */
+	scoped_guard(spinlock_irqsave, &pipe->irqlock) {
+		pipe->state = VSP1_PIPELINE_STOPPED;
+	}
+
+	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
+		vspx_pipe->processing = false;
+	}
+
+	/* Try schedule a new job from the queue. */
+	vsp1_isp_job_run(vspx->vsp1->dev);
+}
+
+/* -----------------------------------------------------------------------------
+ * ISP Driver API (include/media/vsp1.h)
+ */
+
+/**
+ * vsp1_isp_init() - Initialize the VSPX
+ *
+ * @dev: The VSP1 struct device
+ *
+ * Return: %0 on success or a negative error code on failure
+ */
+int vsp1_isp_init(struct device *dev)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+
+	if (!vsp1)
+		return -EPROBE_DEFER;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vsp1_isp_init);
+
+/**
+ * vsp1_isp_get_bus_master - Get VSPX bus master
+ *
+ * The VSPX access memory through an FCPX instance. When allocating memory
+ * buffers that will have to be accessed by the VSPX the 'struct device' of
+ * the FCPX should be used. Use this function to get a reference to it.
+ *
+ * @dev: The VSP1 struct device
+ *
+ * Return: a pointer to the bus master's device
+ */
+struct device *vsp1_isp_get_bus_master(struct device *dev)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+
+	if (!vsp1)
+		return ERR_PTR(-ENODEV);
+
+	return vsp1->bus_master;
+}
+EXPORT_SYMBOL_GPL(vsp1_isp_get_bus_master);
+
+/**
+ * vsp1_isp_alloc_buffers - Allocate buffers in the VSPX address space
+ *
+ * Allocate buffers that will be later accessed by the VSPX.
+ *
+ * @dev: The VSP1 struct device
+ * @size: The size of the buffer to be allocated by the VSPX
+ * @buffer_desc: The allocated buffer description, will be filled with the
+ *		 buffer CPU-mapped address and the bus address
+ *
+ * Return: %0 on success or a negative error code on failure
+ */
+int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
+			   struct vsp1_isp_buffer_desc *buffer_desc)
+{
+	struct device *bus_master = vsp1_isp_get_bus_master(dev);
+
+	if (IS_ERR_OR_NULL(bus_master))
+		return -ENODEV;
+
+	buffer_desc->cpu_addr = dma_alloc_coherent(bus_master, size,
+						   &buffer_desc->dma_addr,
+						   GFP_KERNEL);
+	if (IS_ERR_OR_NULL(buffer_desc->cpu_addr))
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vsp1_isp_alloc_buffers);
+
+/**
+ * vsp1_isp_configure - Configure the VSPX with the RAW image format
+ *
+ * Apply to the VSPX RPF/WPF the size of the RAW image that will be transferred
+ * to the ISP.
+ *
+ * @dev: The VSP1 struct device
+ * @fmt: The RAW image format description
+ *
+ * Return: %0 on success or a negative error code on failure
+ */
+int vsp1_isp_configure(struct device *dev,
+		       const struct v4l2_pix_format_mplane *fmt)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct vsp1_vspx_pipeline *vspx_pipe;
+	struct vsp1_pipeline *pipe;
+	int ret;
+
+	vspx_pipe = &vsp1->vspx->pipe;
+	pipe = &vspx_pipe->pipe;
+
+	/*
+	 * Apply the same format to the RPF0 and WPF0 so that the partition
+	 * calculation results in a single partition.
+	 */
+	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->inputs[0],
+					    fmt->pixelformat, fmt->width,
+					    fmt->height);
+	if (ret)
+		return ret;
+
+	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->output, fmt->pixelformat,
+					    fmt->width, fmt->height);
+	if (ret)
+		return ret;
+
+	vsp1_pipeline_calculate_partition(pipe, &pipe->part_table[0],
+					  fmt->width, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vsp1_isp_configure);
+
+static void vsp1_vspx_release_jobs(struct vsp1_device *vsp1)
+{
+	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
+	struct vsp1_vspx_job *job, *tmp;
+
+	guard(spinlock_irqsave)(&vspx_pipe->jobs_lock);
+
+	list_for_each_entry_safe(job, tmp, &vspx_pipe->jobs, job_queue) {
+		list_del(&job->job_queue);
+		vsp1_dl_list_put(job->dl);
+		kfree(job);
+	}
+}
+
+/**
+ * vsp1_isp_start_streaming - Start processing VSPX jobs
+ *
+ * Start the VSPX and prepare for accepting buffer transfer job requests.
+ *
+ * @dev: The VSP1 struct device
+ * @frame_end: The frame end callback description
+ * @enable: The enable/disable streaming flag
+ *
+ * Return: %0 on success or a negative error code on failure
+ */
+int vsp1_isp_start_streaming(struct device *dev,
+			     struct vsp1_vspx_frame_end *frame_end,
+			     bool enable)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
+	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
+	u32 value;
+	int ret;
+
+	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
+		if (vspx_pipe->enabled == enable)
+			return 0;
+
+		vspx_pipe->enabled = enable;
+	}
+
+	if (!enable) {
+		pipe->state = VSP1_PIPELINE_STOPPED;
+		vsp1_pipeline_stop(pipe);
+		vsp1_vspx_release_jobs(vsp1);
+		vspx_pipe->processing = false;
+		vspx_pipe->vspx_frame_end = NULL;
+		vsp1_dlm_reset(pipe->output->dlm);
+		vsp1_device_put(vsp1);
+		return 0;
+	}
+
+	if (!frame_end) {
+		ret = -EINVAL;
+		goto error_stop_pipe;
+	}
+
+	vspx_pipe->vspx_frame_end = frame_end->vspx_frame_end;
+	vspx_pipe->frame_end_data = frame_end->frame_end_data;
+
+	/* Make sure VSPX is not active. */
+	value = vsp1_read(vsp1, VI6_CMD(0));
+	if (value & VI6_CMD_STRCMD) {
+		dev_err(vsp1->dev,
+			"%s: Starting of WPF0 already reserved\n", __func__);
+		ret = -EBUSY;
+		goto error_stop_pipe;
+	}
+
+	value = vsp1_read(vsp1, VI6_STATUS);
+	if (value & VI6_STATUS_SYS_ACT(0)) {
+		dev_err(vsp1->dev,
+			"%s: WPF0 has not entered idle state\n", __func__);
+		ret = -EBUSY;
+		goto error_stop_pipe;
+	}
+
+	/* Enable the VSP1 and prepare for streaming. */
+	vsp1_pipeline_dump(pipe, "VSPX job");
+
+	ret = vsp1_device_get(vsp1);
+	if (ret < 0)
+		goto error_stop_pipe;
+
+	return 0;
+
+error_stop_pipe:
+	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
+		vspx_pipe->enabled = false;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(vsp1_isp_start_streaming);
+
+/**
+ * vsp1_vspx_job_prepare - Prepare a display list with the content of the buffer
+ *
+ * @dev: The VSP1 struct device
+ * @job: The job description
+ *
+ * Return: %0 on success or a negative error code on failure
+ */
+int vsp1_isp_job_prepare(struct device *dev,
+			 const struct vsp1_isp_job_desc *desc)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
+	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
+	const struct v4l2_pix_format_mplane *pix_mp;
+	struct vsp1_dl_list *second_dl = NULL;
+	struct vsp1_vspx_job *job;
+	struct vsp1_dl_body *dlb;
+	struct vsp1_dl_list *dl;
+	int ret;
+
+	/*
+	 * Populate a display list and append it to the jobs queue.
+	 * Memory is released when the job is consumed.
+	 */
+	job = kmalloc(sizeof(*job), GFP_KERNEL);
+	if (!job)
+		return -ENOMEM;
+
+	/*
+	 * Transfer the buffers described in the job: (optional) ConfigDMA and
+	 * RAW image.
+	 */
+
+	job->dl = vsp1_dl_list_get(pipe->output->dlm);
+	if (!job->dl) {
+		ret = -ENOMEM;
+		goto error_free_job;
+	}
+
+	dl = job->dl;
+	dlb = vsp1_dl_list_get_body0(dl);
+
+	/* Disable RPF1. */
+	vsp1_dl_body_write(dlb, vsp1->rpf[1]->entity.route->reg,
+			   VI6_DPR_NODE_UNUSED);
+
+	/* Configure IIF routing and enable IIF function */
+	vsp1_entity_route_setup(pipe->iif, pipe, dlb);
+	vsp1_entity_configure_stream(pipe->iif, pipe->iif->state, pipe,
+				     dl, dlb);
+
+	/* Configure WPF0 to enable RPF0 as source*/
+	vsp1_entity_route_setup(&pipe->output->entity, pipe, dlb);
+	vsp1_entity_configure_stream(&pipe->output->entity,
+				     pipe->output->entity.state, pipe,
+				     dl, dlb);
+
+	if (desc->config.pairs) {
+		/*
+		 * Configure RPF0 for config data. Transfer the number of
+		 * configuration pairs plus 2 words for the header.
+		 */
+		ret = vsp1_vspx_rpf0_configure(vsp1, desc->config.mem,
+					       V4L2_PIX_FMT_XBGR32,
+					       desc->config.pairs * 2 + 2, 1,
+					       desc->config.pairs * 2 + 2,
+					       VSPX_IIF_SINK_PAD_CONFIG,
+					       dl, dlb);
+		if (ret)
+			goto error_put_dl;
+
+		second_dl = vsp1_dl_list_get(pipe->output->dlm);
+		if (!second_dl) {
+			ret = -ENOMEM;
+			goto error_put_dl;
+		}
+
+		dl = second_dl;
+		dlb = vsp1_dl_list_get_body0(dl);
+	}
+
+	/* Configure RPF0 for RAW image transfer. */
+	pix_mp = &desc->img.fmt.fmt.pix_mp;
+	ret = vsp1_vspx_rpf0_configure(vsp1, desc->img.mem,
+				       pix_mp->pixelformat,
+				       pix_mp->width, pix_mp->height,
+				       pix_mp->plane_fmt[0].bytesperline,
+				       VSPX_IIF_SINK_PAD_IMG, dl, dlb);
+	if (ret)
+		goto error_put_dl;
+
+	if (second_dl)
+		vsp1_dl_list_add_chain(job->dl, second_dl);
+
+	scoped_guard(spinlock_irqsave, &vspx_pipe->jobs_lock) {
+		list_add_tail(&job->job_queue, &vspx_pipe->jobs);
+	}
+
+	return 0;
+
+error_put_dl:
+	if (second_dl)
+		vsp1_dl_list_put(second_dl);
+	vsp1_dl_list_put(job->dl);
+error_free_job:
+	kfree(job);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(vsp1_isp_job_prepare);
+
+/**
+ * vsp1_isp_job_run - Run a buffer transfer on behalf of the ISP
+ *
+ * @dev: The VSP1 struct device
+ */
+void vsp1_isp_job_run(struct device *dev)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
+	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
+	struct vsp1_vspx_job *job;
+
+	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
+
+		if (vspx_pipe->processing)
+			return;
+
+		/* Extract one job, if available, from the jobs list. */
+		scoped_guard(spinlock_irqsave, &vspx_pipe->jobs_lock) {
+			job = list_first_entry_or_null(&vspx_pipe->jobs,
+						       struct vsp1_vspx_job,
+						       job_queue);
+			if (!job)
+				return;
+
+			list_del(&job->job_queue);
+		}
+
+		vspx_pipe->processing = true;
+		vsp1_dl_list_commit(job->dl, 0);
+		kfree(job);
+	}
+
+	/* Trigger VSPX processing by setting VI6_CMD[STRCMD]. */
+	scoped_guard(spinlock_irqsave, &pipe->irqlock) {
+		vsp1_pipeline_run(pipe);
+	}
+}
+EXPORT_SYMBOL_GPL(vsp1_isp_job_run);
+
+/* -----------------------------------------------------------------------------
+ * Initialization and cleanup
+ */
+
+int vsp1_vspx_init(struct vsp1_device *vsp1)
+{
+	struct vsp1_vspx_pipeline *vspx_pipe;
+	struct vsp1_pipeline *pipe;
+
+	vsp1->vspx = devm_kzalloc(vsp1->dev, sizeof(*vsp1->vspx), GFP_KERNEL);
+	if (!vsp1->vspx)
+		return -ENOMEM;
+
+	vsp1->vspx->vsp1 = vsp1;
+
+	vspx_pipe = &vsp1->vspx->pipe;
+	vspx_pipe->processing = false;
+	vspx_pipe->enabled = false;
+
+	pipe = &vspx_pipe->pipe;
+
+	vsp1_pipeline_init(pipe);
+
+	pipe->partitions = 1;
+	pipe->part_table = &vspx_pipe->partition;
+	pipe->interlaced = false;
+	pipe->frame_end = vsp1_vspx_pipeline_frame_end;
+
+	INIT_LIST_HEAD(&vspx_pipe->jobs);
+	spin_lock_init(&vspx_pipe->vspx_lock);
+	spin_lock_init(&vspx_pipe->jobs_lock);
+
+	/*
+	 * Initialize RPF0 as inputs for VSPX and use it unconditionally for
+	 * now.
+	 */
+	pipe->inputs[0] = vsp1->rpf[0];
+	pipe->inputs[0]->entity.pipe = pipe;
+	pipe->inputs[0]->entity.sink = &vsp1->iif->entity;
+	vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->inputs[0],
+				      vspx_default_fmt.pixelformat,
+				      vspx_default_fmt.width,
+				      vspx_default_fmt.height);
+	list_add(&pipe->inputs[0]->entity.list_pipe, &pipe->entities);
+
+	pipe->iif = &vsp1->iif->entity;
+	pipe->iif->pipe = pipe;
+	pipe->iif->sink = &vsp1->wpf[0]->entity;
+	list_add(&pipe->iif->list_pipe, &pipe->entities);
+
+	pipe->output = vsp1->wpf[0];
+	pipe->output->entity.pipe = pipe;
+	vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->output,
+				      vspx_default_fmt.pixelformat,
+				      vspx_default_fmt.width,
+				      vspx_default_fmt.height);
+	list_add(&pipe->output->entity.list_pipe, &pipe->entities);
+
+	return 0;
+}
+
+void vsp1_vspx_cleanup(struct vsp1_device *vsp1)
+{
+	struct vsp1_vspx_pipeline *vspx_pipe;
+
+	vspx_pipe = &vsp1->vspx->pipe;
+
+	vsp1_vspx_release_jobs(vsp1);
+}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_vspx.h b/drivers/media/platform/renesas/vsp1/vsp1_vspx.h
new file mode 100644
index 000000000000..2f68f043075e
--- /dev/null
+++ b/drivers/media/platform/renesas/vsp1/vsp1_vspx.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * vsp1_vspx.h  --  R-Car Gen 4 VSPX
+ *
+ * Copyright (C) 2025 Ideas On Board Oy
+ * Copyright (C) 2025 Renesas Electronics Corporation
+ */
+#ifndef __VSP1_VSPX_H__
+#define __VSP1_VSPX_H__
+
+#include <linux/container_of.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+
+#include <media/vsp1.h>
+
+#include "vsp1.h"
+#include "vsp1_pipe.h"
+
+/* Pixel format for ConfigDMA buffers. */
+#define V4L2_META_FMT_RCAR_V4H	v4l2_fourcc('R', 'C', 'A', '4')
+
+/*
+ * struct vsp1_vspx_pipeline - VSPX pipeline
+ * @pipe: the VSP1 pipeline
+ * @partition: the pre-calculated partition used by the pipeline
+ * @vspx_lock: protect access to the VSPX configuration
+ * @processing: VSPX busy flag
+ * @jobs_lock: protect the jobs queue
+ * @jobs: jobs queue
+ * @vspx_frame_end: frame end callback
+ * @frame_end_data: data for the frame end callback
+ */
+struct vsp1_vspx_pipeline {
+	struct vsp1_pipeline pipe;
+	struct vsp1_partition partition;
+
+	/* Protects the pipeline configuration */
+	spinlock_t vspx_lock;
+	bool processing;
+	bool enabled;
+
+	/* Protects the jobs list */
+	spinlock_t jobs_lock;
+	struct list_head jobs;
+
+	void (*vspx_frame_end)(void *frame_end_data);
+	void *frame_end_data;
+};
+
+static inline struct vsp1_vspx_pipeline *
+to_vsp1_vspx_pipeline(struct vsp1_pipeline *pipe)
+{
+	return container_of(pipe, struct vsp1_vspx_pipeline, pipe);
+}
+
+/*
+ * struct vsp1_vspx_job - VSPX transfer job
+ * @dl: Display list populated by vsp1_isp_job_prepare
+ * @job_queue: List handle
+ */
+struct vsp1_vspx_job {
+	struct vsp1_dl_list *dl;
+	struct list_head job_queue;
+};
+
+/*
+ * struct vsp1_vspx - VSPX device
+ * @vsp1: the VSP1 device
+ * @pipe: the VSPX pipeline
+ */
+struct vsp1_vspx {
+	struct vsp1_device *vsp1;
+	struct vsp1_vspx_pipeline pipe;
+};
+
+static inline struct vsp1_vspx *
+to_vsp1_vspx(struct vsp1_vspx_pipeline *vspx_pipe)
+{
+	return container_of(vspx_pipe, struct vsp1_vspx, pipe);
+}
+
+int vsp1_vspx_init(struct vsp1_device *vsp1);
+void vsp1_vspx_cleanup(struct vsp1_device *vsp1);
+
+#endif /* __VSP1_VSPX_H__ */
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 48f4a5023d81..68e4d9891dff 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -9,12 +9,17 @@
 #ifndef __MEDIA_VSP1_H__
 #define __MEDIA_VSP1_H__
 
+#include <linux/list.h>
 #include <linux/scatterlist.h>
 #include <linux/types.h>
 #include <linux/videodev2.h>
 
 struct device;
 
+/*******************************************************************************
+ * VSP1 DU interface
+ */
+
 int vsp1_du_init(struct device *dev);
 
 #define VSP1_DU_STATUS_COMPLETE		BIT(0)
@@ -117,4 +122,72 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
 int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt);
 void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt);
 
+/*******************************************************************************
+ * VSP1 ISP interface
+ */
+
+/**
+ * struct vsp1_isp_buffer_desc - Describe a buffer allocated by VSPX
+ *
+ * @cpu_addr: CPU-mapped address of a buffer allocated by VSPX
+ * @dma_addr: bus address of a buffer allocated by VSPX
+ */
+struct vsp1_isp_buffer_desc {
+	void *cpu_addr;
+	dma_addr_t dma_addr;
+};
+
+/**
+ * struct vsp1_isp_job_desc - Describe a VSPX buffer transfer request
+ *
+ * Describe a transfer request for the VSPX to perform on behalf of the ISP.
+ * The transfer job descriptor contains an optional ConfigDMA buffer and
+ * one RAW image buffer. Set config.pairs to 0 if no ConfigDMA buffer should
+ * be transferred.
+ *
+ * @config: ConfigDMA buffer
+ * @config.pairs: number of reg-value pairs in the ConfigDMA buffer
+ * @config.mem: bus address of the ConfigDMA buffer
+ * @img: RAW image buffer
+ * @img.fmt: RAW image format
+ * @img.mem: bus address of the RAW image buffer
+ */
+struct vsp1_isp_job_desc {
+	struct {
+		unsigned int pairs;
+		dma_addr_t mem;
+	} config;
+	struct {
+		struct v4l2_format fmt;
+		dma_addr_t mem;
+	} img;
+};
+
+/**
+ * struct vsp1_vspx_frame_end - VSPX frame end callback data
+ *
+ * @vspx_frame_end: Frame end callback. Called after a transfer job has been
+ *		    completed. If the job includes both a ConfigDMA and a
+ *		    RAW image, the callback is called after both have been
+ *		    transferred
+ * @frame_end_data: Frame end callback data, passed to vspx_frame_end
+ */
+struct vsp1_vspx_frame_end {
+	void (*vspx_frame_end)(void *data);
+	void *frame_end_data;
+};
+
+int vsp1_isp_init(struct device *dev);
+struct device *vsp1_isp_get_bus_master(struct device *dev);
+int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
+			   struct vsp1_isp_buffer_desc *buffer_desc);
+int vsp1_isp_configure(struct device *dev,
+		       const struct v4l2_pix_format_mplane *fmt);
+int vsp1_isp_start_streaming(struct device *dev,
+			     struct vsp1_vspx_frame_end *frame_end,
+			     bool enable);
+int vsp1_isp_job_prepare(struct device *dev,
+			 const struct vsp1_isp_job_desc *desc);
+void vsp1_isp_job_run(struct device *dev);
+
 #endif /* __MEDIA_VSP1_H__ */

-- 
2.48.1

Re: [PATCH v7 5/5] media: vsp1: Add VSPX support

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2025-04-01 16:22:05 +0200, Jacopo Mondi wrote:
> Add support for VSPX, a specialized version of the VSP2 that
> transfer data to the ISP. The VSPX is composed by two RPF units
> to read data from external memory and an IIF instance that performs
> transfer towards the ISP.
> 
> The VSPX is supported through a newly introduced vsp1_vspx.c file that
> exposes two interfaces: vsp1_vspx interface, declared in vsp1_vspx.h
> for the vsp1 core to initialize and cleanup the VSPX, and a vsp1_isp
> interface, declared in include/media/vsp1.h for the ISP driver to
> control the VSPX operations.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/vsp1/Makefile    |   1 +
>  drivers/media/platform/renesas/vsp1/vsp1.h      |   1 +
>  drivers/media/platform/renesas/vsp1/vsp1_drv.c  |  13 +-
>  drivers/media/platform/renesas/vsp1/vsp1_regs.h |   1 +
>  drivers/media/platform/renesas/vsp1/vsp1_vspx.c | 604 ++++++++++++++++++++++++
>  drivers/media/platform/renesas/vsp1/vsp1_vspx.h |  86 ++++
>  include/media/vsp1.h                            |  73 +++
>  7 files changed, 778 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/Makefile b/drivers/media/platform/renesas/vsp1/Makefile
> index de8c802e1d1a..2057c8f7be47 100644
> --- a/drivers/media/platform/renesas/vsp1/Makefile
> +++ b/drivers/media/platform/renesas/vsp1/Makefile
> @@ -6,5 +6,6 @@ vsp1-y					+= vsp1_clu.o vsp1_hsit.o vsp1_lut.o
>  vsp1-y					+= vsp1_brx.o vsp1_sru.o vsp1_uds.o
>  vsp1-y					+= vsp1_hgo.o vsp1_hgt.o vsp1_histo.o
>  vsp1-y					+= vsp1_iif.o vsp1_lif.o vsp1_uif.o
> +vsp1-y					+= vsp1_vspx.o
>  
>  obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1.o
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1.h b/drivers/media/platform/renesas/vsp1/vsp1.h
> index 263024639dd2..1872e403f26b 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1.h
> @@ -110,6 +110,7 @@ struct vsp1_device {
>  	struct media_entity_operations media_ops;
>  
>  	struct vsp1_drm *drm;
> +	struct vsp1_vspx *vspx;
>  };
>  
>  int vsp1_device_get(struct vsp1_device *vsp1);
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> index d13e9b31aa7c..e338ab8af292 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> @@ -38,6 +38,7 @@
>  #include "vsp1_uds.h"
>  #include "vsp1_uif.h"
>  #include "vsp1_video.h"
> +#include "vsp1_vspx.h"
>  
>  /* -----------------------------------------------------------------------------
>   * Interrupt Handling
> @@ -488,7 +489,10 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
>  
>  		ret = media_device_register(mdev);
>  	} else {
> -		ret = vsp1_drm_init(vsp1);
> +		if (vsp1->info->version == VI6_IP_VERSION_MODEL_VSPX_GEN4)
> +			ret = vsp1_vspx_init(vsp1);
> +		else
> +			ret = vsp1_drm_init(vsp1);
>  	}
>  
>  done:
> @@ -846,6 +850,13 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
>  		.uif_count = 2,
>  		.wpf_count = 1,
>  		.num_bru_inputs = 5,
> +	}, {
> +		.version = VI6_IP_VERSION_MODEL_VSPX_GEN4,
> +		.model = "VSP2-X",
> +		.gen = 4,
> +		.features = VSP1_HAS_IIF,
> +		.rpf_count = 2,
> +		.wpf_count = 1,
>  	},
>  };
>  
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_regs.h b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> index 86e47c2d991f..10cfbcd1b6e0 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> @@ -799,6 +799,7 @@
>  #define VI6_IP_VERSION_MODEL_VSPDL_GEN3	(0x19 << 8)
>  #define VI6_IP_VERSION_MODEL_VSPBS_GEN3	(0x1a << 8)
>  #define VI6_IP_VERSION_MODEL_VSPD_GEN4	(0x1c << 8)
> +#define VI6_IP_VERSION_MODEL_VSPX_GEN4	(0x1d << 8)
>  /* RZ/G2L SoCs have no version register, So use 0x80 as the model version */
>  #define VI6_IP_VERSION_MODEL_VSPD_RZG2L	(0x80 << 8)
>  
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_vspx.c b/drivers/media/platform/renesas/vsp1/vsp1_vspx.c
> new file mode 100644
> index 000000000000..db9707f2b532
> --- /dev/null
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_vspx.c
> @@ -0,0 +1,604 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * vsp1_vspx.c  --  R-Car Gen 4 VSPX
> + *
> + * Copyright (C) 2025 Ideas On Board Oy
> + * Copyright (C) 2025 Renesas Electronics Corporation
> + */
> +
> +#include "vsp1_vspx.h"
> +
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/slab.h>
> +
> +#include <media/media-entity.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/vsp1.h>
> +
> +#include "vsp1.h"
> +#include "vsp1_dl.h"
> +#include "vsp1_iif.h"
> +#include "vsp1_pipe.h"
> +#include "vsp1_rwpf.h"
> +
> +static const struct v4l2_pix_format_mplane vspx_default_fmt = {
> +	.width = 1920,
> +	.height = 1080,
> +	.pixelformat = V4L2_PIX_FMT_SRGGB8,
> +	.field = V4L2_FIELD_NONE,
> +	.num_planes = 1,
> +	.plane_fmt = {
> +		[0] = {
> +			.sizeimage = 1920 * 1080,
> +			.bytesperline = 1920,
> +		},
> +	},
> +};
> +
> +/*
> + * Apply the given width, height and fourcc to the subdevice inside the
> + * VSP1 entity.
> + */
> +static int vsp1_vspx_rwpf_set_subdev_fmt(struct vsp1_device *vsp1,
> +					 struct vsp1_rwpf *rwpf,
> +					 u32 isp_fourcc,
> +					 unsigned int width,
> +					 unsigned int height)
> +{
> +	struct vsp1_entity *ent = &rwpf->entity;
> +	const struct vsp1_format_info *fmtinfo;
> +	struct v4l2_subdev_format format = {};
> +	u32 vspx_fourcc;
> +	int ret;
> +
> +	switch (isp_fourcc) {
> +	case V4L2_PIX_FMT_GREY:
> +		/* 8 bit RAW Bayer */
> +		vspx_fourcc = V4L2_PIX_FMT_RGB332;
> +		break;
> +	case V4L2_PIX_FMT_Y10:
> +	case V4L2_PIX_FMT_Y12:
> +	case V4L2_PIX_FMT_Y16:
> +		/* 10, 12 and 16 bit RAW Bayer */
> +		vspx_fourcc = V4L2_PIX_FMT_RGB565;
> +		break;
> +	case V4L2_PIX_FMT_XBGR32:
> +		/* ConfigDMA */
> +		vspx_fourcc = V4L2_PIX_FMT_XBGR32;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	fmtinfo = vsp1_get_format_info(vsp1, vspx_fourcc);
> +	if (!fmtinfo) {
> +		dev_dbg(vsp1->dev, "Unsupported pixel format %08x\n",
> +			vspx_fourcc);
> +		return -EINVAL;
> +	}
> +
> +	rwpf->fmtinfo = fmtinfo;
> +
> +	format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +	format.pad = RWPF_PAD_SINK;
> +	format.format.width = width;
> +	format.format.height = height;
> +	format.format.field = V4L2_FIELD_NONE;
> +	format.format.code = fmtinfo->mbus;
> +
> +	ret = v4l2_subdev_call(&ent->subdev, pad, set_fmt, NULL, &format);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/*
> + * Configure RPF0 for ConfigDMA or RAW image transfer.
> + */
> +static int vsp1_vspx_rpf0_configure(struct vsp1_device *vsp1,
> +				    dma_addr_t addr, u32 isp_fourcc,
> +				    unsigned int width, unsigned int height,
> +				    unsigned int stride,
> +				    unsigned int iif_sink_pad,
> +				    struct vsp1_dl_list *dl,
> +				    struct vsp1_dl_body *dlb)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> +	struct vsp1_rwpf *rpf0 = pipe->inputs[0];
> +	int ret;
> +
> +	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, rpf0, isp_fourcc, width,
> +					    height);
> +	if (ret)
> +		return ret;
> +
> +	rpf0->format.plane_fmt[0].bytesperline = stride;
> +
> +	/*
> +	 * Connect RPF0 to the IIF sink pad corresponding to the config or image
> +	 * path.
> +	 */
> +	rpf0->entity.sink_pad = iif_sink_pad;
> +
> +	pipe->part_table[0].rpf[0].width = width;
> +	pipe->part_table[0].rpf[0].height = height;
> +
> +	rpf0->mem.addr[0] = addr;
> +	rpf0->mem.addr[1] = 0;
> +	rpf0->mem.addr[2] = 0;
> +
> +	vsp1_entity_route_setup(&rpf0->entity, pipe, dlb);
> +	vsp1_entity_configure_stream(&rpf0->entity, rpf0->entity.state, pipe,
> +				     dl, dlb);
> +	vsp1_entity_configure_partition(&rpf0->entity, pipe,
> +					&pipe->part_table[0], dl, dlb);
> +
> +	return 0;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Interrupt handling
> + */
> +static void vsp1_vspx_pipeline_frame_end(struct vsp1_pipeline *pipe,
> +					 unsigned int completion)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe = to_vsp1_vspx_pipeline(pipe);
> +	struct vsp1_vspx *vspx = to_vsp1_vspx(vspx_pipe);
> +
> +	if (vspx_pipe->vspx_frame_end)
> +		vspx_pipe->vspx_frame_end(vspx_pipe->frame_end_data);
> +
> +	/*
> +	 * Set the pipeline state to stopped to ensure the next call to
> +	 * vsp1_pipeline_run() actually starts the VSPX.
> +	 */
> +	scoped_guard(spinlock_irqsave, &pipe->irqlock) {
> +		pipe->state = VSP1_PIPELINE_STOPPED;
> +	}
> +
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> +		vspx_pipe->processing = false;
> +	}
> +
> +	/* Try schedule a new job from the queue. */
> +	vsp1_isp_job_run(vspx->vsp1->dev);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * ISP Driver API (include/media/vsp1.h)
> + */
> +
> +/**
> + * vsp1_isp_init() - Initialize the VSPX
> + *
> + * @dev: The VSP1 struct device
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_init(struct device *dev)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +
> +	if (!vsp1)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_init);
> +
> +/**
> + * vsp1_isp_get_bus_master - Get VSPX bus master
> + *
> + * The VSPX access memory through an FCPX instance. When allocating memory
> + * buffers that will have to be accessed by the VSPX the 'struct device' of
> + * the FCPX should be used. Use this function to get a reference to it.
> + *
> + * @dev: The VSP1 struct device
> + *
> + * Return: a pointer to the bus master's device
> + */
> +struct device *vsp1_isp_get_bus_master(struct device *dev)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +
> +	if (!vsp1)
> +		return ERR_PTR(-ENODEV);
> +
> +	return vsp1->bus_master;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_get_bus_master);
> +
> +/**
> + * vsp1_isp_alloc_buffers - Allocate buffers in the VSPX address space
> + *
> + * Allocate buffers that will be later accessed by the VSPX.
> + *
> + * @dev: The VSP1 struct device
> + * @size: The size of the buffer to be allocated by the VSPX
> + * @buffer_desc: The allocated buffer description, will be filled with the
> + *		 buffer CPU-mapped address and the bus address
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> +			   struct vsp1_isp_buffer_desc *buffer_desc)
> +{
> +	struct device *bus_master = vsp1_isp_get_bus_master(dev);
> +
> +	if (IS_ERR_OR_NULL(bus_master))
> +		return -ENODEV;
> +
> +	buffer_desc->cpu_addr = dma_alloc_coherent(bus_master, size,
> +						   &buffer_desc->dma_addr,
> +						   GFP_KERNEL);
> +	if (IS_ERR_OR_NULL(buffer_desc->cpu_addr))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_alloc_buffers);
> +
> +/**
> + * vsp1_isp_configure - Configure the VSPX with the RAW image format
> + *
> + * Apply to the VSPX RPF/WPF the size of the RAW image that will be transferred
> + * to the ISP.
> + *
> + * @dev: The VSP1 struct device
> + * @fmt: The RAW image format description
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_configure(struct device *dev,
> +		       const struct v4l2_pix_format_mplane *fmt)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_vspx_pipeline *vspx_pipe;
> +	struct vsp1_pipeline *pipe;
> +	int ret;
> +
> +	vspx_pipe = &vsp1->vspx->pipe;
> +	pipe = &vspx_pipe->pipe;
> +
> +	/*
> +	 * Apply the same format to the RPF0 and WPF0 so that the partition
> +	 * calculation results in a single partition.
> +	 */
> +	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->inputs[0],
> +					    fmt->pixelformat, fmt->width,
> +					    fmt->height);
> +	if (ret)
> +		return ret;
> +
> +	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->output, fmt->pixelformat,
> +					    fmt->width, fmt->height);
> +	if (ret)
> +		return ret;
> +
> +	vsp1_pipeline_calculate_partition(pipe, &pipe->part_table[0],
> +					  fmt->width, 0);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_configure);
> +
> +static void vsp1_vspx_release_jobs(struct vsp1_device *vsp1)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_vspx_job *job, *tmp;
> +
> +	guard(spinlock_irqsave)(&vspx_pipe->jobs_lock);
> +
> +	list_for_each_entry_safe(job, tmp, &vspx_pipe->jobs, job_queue) {
> +		list_del(&job->job_queue);
> +		vsp1_dl_list_put(job->dl);
> +		kfree(job);
> +	}
> +}
> +
> +/**
> + * vsp1_isp_start_streaming - Start processing VSPX jobs
> + *
> + * Start the VSPX and prepare for accepting buffer transfer job requests.
> + *
> + * @dev: The VSP1 struct device
> + * @frame_end: The frame end callback description
> + * @enable: The enable/disable streaming flag
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_start_streaming(struct device *dev,
> +			     struct vsp1_vspx_frame_end *frame_end,
> +			     bool enable)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> +	u32 value;
> +	int ret;
> +
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> +		if (vspx_pipe->enabled == enable)
> +			return 0;
> +
> +		vspx_pipe->enabled = enable;
> +	}
> +
> +	if (!enable) {
> +		pipe->state = VSP1_PIPELINE_STOPPED;
> +		vsp1_pipeline_stop(pipe);
> +		vsp1_vspx_release_jobs(vsp1);
> +		vspx_pipe->processing = false;
> +		vspx_pipe->vspx_frame_end = NULL;
> +		vsp1_dlm_reset(pipe->output->dlm);
> +		vsp1_device_put(vsp1);
> +		return 0;
> +	}
> +
> +	if (!frame_end) {
> +		ret = -EINVAL;
> +		goto error_stop_pipe;
> +	}
> +
> +	vspx_pipe->vspx_frame_end = frame_end->vspx_frame_end;
> +	vspx_pipe->frame_end_data = frame_end->frame_end_data;
> +
> +	/* Make sure VSPX is not active. */
> +	value = vsp1_read(vsp1, VI6_CMD(0));
> +	if (value & VI6_CMD_STRCMD) {
> +		dev_err(vsp1->dev,
> +			"%s: Starting of WPF0 already reserved\n", __func__);
> +		ret = -EBUSY;
> +		goto error_stop_pipe;
> +	}
> +
> +	value = vsp1_read(vsp1, VI6_STATUS);
> +	if (value & VI6_STATUS_SYS_ACT(0)) {
> +		dev_err(vsp1->dev,
> +			"%s: WPF0 has not entered idle state\n", __func__);
> +		ret = -EBUSY;
> +		goto error_stop_pipe;
> +	}
> +
> +	/* Enable the VSP1 and prepare for streaming. */
> +	vsp1_pipeline_dump(pipe, "VSPX job");
> +
> +	ret = vsp1_device_get(vsp1);
> +	if (ret < 0)
> +		goto error_stop_pipe;
> +
> +	return 0;
> +
> +error_stop_pipe:
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> +		vspx_pipe->enabled = false;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_start_streaming);
> +
> +/**
> + * vsp1_vspx_job_prepare - Prepare a display list with the content of the buffer
> + *
> + * @dev: The VSP1 struct device
> + * @job: The job description
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_job_prepare(struct device *dev,
> +			 const struct vsp1_isp_job_desc *desc)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> +	const struct v4l2_pix_format_mplane *pix_mp;
> +	struct vsp1_dl_list *second_dl = NULL;
> +	struct vsp1_vspx_job *job;
> +	struct vsp1_dl_body *dlb;
> +	struct vsp1_dl_list *dl;
> +	int ret;
> +
> +	/*
> +	 * Populate a display list and append it to the jobs queue.
> +	 * Memory is released when the job is consumed.
> +	 */
> +	job = kmalloc(sizeof(*job), GFP_KERNEL);
> +	if (!job)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Transfer the buffers described in the job: (optional) ConfigDMA and
> +	 * RAW image.
> +	 */
> +
> +	job->dl = vsp1_dl_list_get(pipe->output->dlm);
> +	if (!job->dl) {
> +		ret = -ENOMEM;
> +		goto error_free_job;
> +	}
> +
> +	dl = job->dl;
> +	dlb = vsp1_dl_list_get_body0(dl);
> +
> +	/* Disable RPF1. */
> +	vsp1_dl_body_write(dlb, vsp1->rpf[1]->entity.route->reg,
> +			   VI6_DPR_NODE_UNUSED);
> +
> +	/* Configure IIF routing and enable IIF function */
> +	vsp1_entity_route_setup(pipe->iif, pipe, dlb);
> +	vsp1_entity_configure_stream(pipe->iif, pipe->iif->state, pipe,
> +				     dl, dlb);
> +
> +	/* Configure WPF0 to enable RPF0 as source*/
> +	vsp1_entity_route_setup(&pipe->output->entity, pipe, dlb);
> +	vsp1_entity_configure_stream(&pipe->output->entity,
> +				     pipe->output->entity.state, pipe,
> +				     dl, dlb);
> +
> +	if (desc->config.pairs) {
> +		/*
> +		 * Configure RPF0 for config data. Transfer the number of
> +		 * configuration pairs plus 2 words for the header.
> +		 */
> +		ret = vsp1_vspx_rpf0_configure(vsp1, desc->config.mem,
> +					       V4L2_PIX_FMT_XBGR32,
> +					       desc->config.pairs * 2 + 2, 1,
> +					       desc->config.pairs * 2 + 2,
> +					       VSPX_IIF_SINK_PAD_CONFIG,
> +					       dl, dlb);
> +		if (ret)
> +			goto error_put_dl;
> +
> +		second_dl = vsp1_dl_list_get(pipe->output->dlm);
> +		if (!second_dl) {
> +			ret = -ENOMEM;
> +			goto error_put_dl;
> +		}
> +
> +		dl = second_dl;
> +		dlb = vsp1_dl_list_get_body0(dl);
> +	}
> +
> +	/* Configure RPF0 for RAW image transfer. */
> +	pix_mp = &desc->img.fmt.fmt.pix_mp;
> +	ret = vsp1_vspx_rpf0_configure(vsp1, desc->img.mem,
> +				       pix_mp->pixelformat,
> +				       pix_mp->width, pix_mp->height,
> +				       pix_mp->plane_fmt[0].bytesperline,
> +				       VSPX_IIF_SINK_PAD_IMG, dl, dlb);
> +	if (ret)
> +		goto error_put_dl;
> +
> +	if (second_dl)
> +		vsp1_dl_list_add_chain(job->dl, second_dl);
> +
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->jobs_lock) {
> +		list_add_tail(&job->job_queue, &vspx_pipe->jobs);
> +	}
> +
> +	return 0;
> +
> +error_put_dl:
> +	if (second_dl)
> +		vsp1_dl_list_put(second_dl);
> +	vsp1_dl_list_put(job->dl);
> +error_free_job:
> +	kfree(job);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_job_prepare);
> +
> +/**
> + * vsp1_isp_job_run - Run a buffer transfer on behalf of the ISP
> + *
> + * @dev: The VSP1 struct device
> + */
> +void vsp1_isp_job_run(struct device *dev)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> +	struct vsp1_vspx_job *job;
> +
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> +
> +		if (vspx_pipe->processing)
> +			return;
> +
> +		/* Extract one job, if available, from the jobs list. */
> +		scoped_guard(spinlock_irqsave, &vspx_pipe->jobs_lock) {
> +			job = list_first_entry_or_null(&vspx_pipe->jobs,
> +						       struct vsp1_vspx_job,
> +						       job_queue);
> +			if (!job)
> +				return;
> +
> +			list_del(&job->job_queue);
> +		}
> +
> +		vspx_pipe->processing = true;
> +		vsp1_dl_list_commit(job->dl, 0);
> +		kfree(job);
> +	}
> +
> +	/* Trigger VSPX processing by setting VI6_CMD[STRCMD]. */
> +	scoped_guard(spinlock_irqsave, &pipe->irqlock) {
> +		vsp1_pipeline_run(pipe);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_job_run);
> +
> +/* -----------------------------------------------------------------------------
> + * Initialization and cleanup
> + */
> +
> +int vsp1_vspx_init(struct vsp1_device *vsp1)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe;
> +	struct vsp1_pipeline *pipe;
> +
> +	vsp1->vspx = devm_kzalloc(vsp1->dev, sizeof(*vsp1->vspx), GFP_KERNEL);
> +	if (!vsp1->vspx)
> +		return -ENOMEM;
> +
> +	vsp1->vspx->vsp1 = vsp1;
> +
> +	vspx_pipe = &vsp1->vspx->pipe;
> +	vspx_pipe->processing = false;
> +	vspx_pipe->enabled = false;
> +
> +	pipe = &vspx_pipe->pipe;
> +
> +	vsp1_pipeline_init(pipe);
> +
> +	pipe->partitions = 1;
> +	pipe->part_table = &vspx_pipe->partition;
> +	pipe->interlaced = false;
> +	pipe->frame_end = vsp1_vspx_pipeline_frame_end;
> +
> +	INIT_LIST_HEAD(&vspx_pipe->jobs);
> +	spin_lock_init(&vspx_pipe->vspx_lock);
> +	spin_lock_init(&vspx_pipe->jobs_lock);
> +
> +	/*
> +	 * Initialize RPF0 as inputs for VSPX and use it unconditionally for
> +	 * now.
> +	 */
> +	pipe->inputs[0] = vsp1->rpf[0];
> +	pipe->inputs[0]->entity.pipe = pipe;
> +	pipe->inputs[0]->entity.sink = &vsp1->iif->entity;
> +	vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->inputs[0],
> +				      vspx_default_fmt.pixelformat,
> +				      vspx_default_fmt.width,
> +				      vspx_default_fmt.height);
> +	list_add(&pipe->inputs[0]->entity.list_pipe, &pipe->entities);
> +
> +	pipe->iif = &vsp1->iif->entity;
> +	pipe->iif->pipe = pipe;
> +	pipe->iif->sink = &vsp1->wpf[0]->entity;
> +	list_add(&pipe->iif->list_pipe, &pipe->entities);
> +
> +	pipe->output = vsp1->wpf[0];
> +	pipe->output->entity.pipe = pipe;
> +	vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->output,
> +				      vspx_default_fmt.pixelformat,
> +				      vspx_default_fmt.width,
> +				      vspx_default_fmt.height);
> +	list_add(&pipe->output->entity.list_pipe, &pipe->entities);
> +
> +	return 0;
> +}
> +
> +void vsp1_vspx_cleanup(struct vsp1_device *vsp1)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe;
> +
> +	vspx_pipe = &vsp1->vspx->pipe;
> +
> +	vsp1_vspx_release_jobs(vsp1);
> +}
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_vspx.h b/drivers/media/platform/renesas/vsp1/vsp1_vspx.h
> new file mode 100644
> index 000000000000..2f68f043075e
> --- /dev/null
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_vspx.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * vsp1_vspx.h  --  R-Car Gen 4 VSPX
> + *
> + * Copyright (C) 2025 Ideas On Board Oy
> + * Copyright (C) 2025 Renesas Electronics Corporation
> + */
> +#ifndef __VSP1_VSPX_H__
> +#define __VSP1_VSPX_H__
> +
> +#include <linux/container_of.h>
> +#include <linux/list.h>
> +#include <linux/spinlock.h>
> +
> +#include <media/vsp1.h>
> +
> +#include "vsp1.h"
> +#include "vsp1_pipe.h"
> +
> +/* Pixel format for ConfigDMA buffers. */
> +#define V4L2_META_FMT_RCAR_V4H	v4l2_fourcc('R', 'C', 'A', '4')
> +
> +/*
> + * struct vsp1_vspx_pipeline - VSPX pipeline
> + * @pipe: the VSP1 pipeline
> + * @partition: the pre-calculated partition used by the pipeline
> + * @vspx_lock: protect access to the VSPX configuration
> + * @processing: VSPX busy flag
> + * @jobs_lock: protect the jobs queue
> + * @jobs: jobs queue
> + * @vspx_frame_end: frame end callback
> + * @frame_end_data: data for the frame end callback
> + */
> +struct vsp1_vspx_pipeline {
> +	struct vsp1_pipeline pipe;
> +	struct vsp1_partition partition;
> +
> +	/* Protects the pipeline configuration */
> +	spinlock_t vspx_lock;
> +	bool processing;
> +	bool enabled;
> +
> +	/* Protects the jobs list */
> +	spinlock_t jobs_lock;
> +	struct list_head jobs;
> +
> +	void (*vspx_frame_end)(void *frame_end_data);
> +	void *frame_end_data;
> +};
> +
> +static inline struct vsp1_vspx_pipeline *
> +to_vsp1_vspx_pipeline(struct vsp1_pipeline *pipe)
> +{
> +	return container_of(pipe, struct vsp1_vspx_pipeline, pipe);
> +}
> +
> +/*
> + * struct vsp1_vspx_job - VSPX transfer job
> + * @dl: Display list populated by vsp1_isp_job_prepare
> + * @job_queue: List handle
> + */
> +struct vsp1_vspx_job {
> +	struct vsp1_dl_list *dl;
> +	struct list_head job_queue;
> +};
> +
> +/*
> + * struct vsp1_vspx - VSPX device
> + * @vsp1: the VSP1 device
> + * @pipe: the VSPX pipeline
> + */
> +struct vsp1_vspx {
> +	struct vsp1_device *vsp1;
> +	struct vsp1_vspx_pipeline pipe;
> +};
> +
> +static inline struct vsp1_vspx *
> +to_vsp1_vspx(struct vsp1_vspx_pipeline *vspx_pipe)
> +{
> +	return container_of(vspx_pipe, struct vsp1_vspx, pipe);
> +}
> +
> +int vsp1_vspx_init(struct vsp1_device *vsp1);
> +void vsp1_vspx_cleanup(struct vsp1_device *vsp1);
> +
> +#endif /* __VSP1_VSPX_H__ */
> diff --git a/include/media/vsp1.h b/include/media/vsp1.h
> index 48f4a5023d81..68e4d9891dff 100644
> --- a/include/media/vsp1.h
> +++ b/include/media/vsp1.h
> @@ -9,12 +9,17 @@
>  #ifndef __MEDIA_VSP1_H__
>  #define __MEDIA_VSP1_H__
>  
> +#include <linux/list.h>
>  #include <linux/scatterlist.h>
>  #include <linux/types.h>
>  #include <linux/videodev2.h>
>  
>  struct device;
>  
> +/*******************************************************************************
> + * VSP1 DU interface
> + */
> +
>  int vsp1_du_init(struct device *dev);
>  
>  #define VSP1_DU_STATUS_COMPLETE		BIT(0)
> @@ -117,4 +122,72 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
>  int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt);
>  void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt);
>  
> +/*******************************************************************************
> + * VSP1 ISP interface
> + */
> +
> +/**
> + * struct vsp1_isp_buffer_desc - Describe a buffer allocated by VSPX
> + *
> + * @cpu_addr: CPU-mapped address of a buffer allocated by VSPX
> + * @dma_addr: bus address of a buffer allocated by VSPX
> + */
> +struct vsp1_isp_buffer_desc {
> +	void *cpu_addr;
> +	dma_addr_t dma_addr;
> +};
> +
> +/**
> + * struct vsp1_isp_job_desc - Describe a VSPX buffer transfer request
> + *
> + * Describe a transfer request for the VSPX to perform on behalf of the ISP.
> + * The transfer job descriptor contains an optional ConfigDMA buffer and
> + * one RAW image buffer. Set config.pairs to 0 if no ConfigDMA buffer should
> + * be transferred.
> + *
> + * @config: ConfigDMA buffer
> + * @config.pairs: number of reg-value pairs in the ConfigDMA buffer
> + * @config.mem: bus address of the ConfigDMA buffer
> + * @img: RAW image buffer
> + * @img.fmt: RAW image format
> + * @img.mem: bus address of the RAW image buffer
> + */
> +struct vsp1_isp_job_desc {
> +	struct {
> +		unsigned int pairs;
> +		dma_addr_t mem;
> +	} config;
> +	struct {
> +		struct v4l2_format fmt;

I wonder if we can't drop this member and use the format passed into 
vsp1_isp_configure()? I think if these two formats ever differed bad 
things would happen no?

> +		dma_addr_t mem;
> +	} img;
> +};
> +
> +/**
> + * struct vsp1_vspx_frame_end - VSPX frame end callback data
> + *
> + * @vspx_frame_end: Frame end callback. Called after a transfer job has been
> + *		    completed. If the job includes both a ConfigDMA and a
> + *		    RAW image, the callback is called after both have been
> + *		    transferred
> + * @frame_end_data: Frame end callback data, passed to vspx_frame_end
> + */
> +struct vsp1_vspx_frame_end {
> +	void (*vspx_frame_end)(void *data);
> +	void *frame_end_data;
> +};
> +
> +int vsp1_isp_init(struct device *dev);
> +struct device *vsp1_isp_get_bus_master(struct device *dev);
> +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> +			   struct vsp1_isp_buffer_desc *buffer_desc);
> +int vsp1_isp_configure(struct device *dev,
> +		       const struct v4l2_pix_format_mplane *fmt);
> +int vsp1_isp_start_streaming(struct device *dev,
> +			     struct vsp1_vspx_frame_end *frame_end,
> +			     bool enable);
> +int vsp1_isp_job_prepare(struct device *dev,
> +			 const struct vsp1_isp_job_desc *desc);
> +void vsp1_isp_job_run(struct device *dev);
> +
>  #endif /* __MEDIA_VSP1_H__ */
> 
> -- 
> 2.48.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v7 3/5] media: vsp1: wpf: Propagate vsp1_rwpf_init_ctrls()

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2025-04-01 16:22:03 +0200, Jacopo Mondi wrote:
> vsp1_wpf.c calls vsp1_rwpf_init_ctrls() to initialize controls that
> are common between RPF and WPF.
> 
> However, the vsp1_wpf.c implementation does not check for the function
> call return value. Fix this by propagating to the caller the return
> value.
> 
> While at it, drop a duplicated error message in wpf_init_controls() as
> the caller already report it.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> 
> ---
> v2->v3:
>   - New patch
> ---
>  drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> index f176750ccd98..da651a882bbb 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> @@ -133,6 +133,7 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
>  {
>  	struct vsp1_device *vsp1 = wpf->entity.vsp1;
>  	unsigned int num_flip_ctrls;
> +	int ret;
>  
>  	spin_lock_init(&wpf->flip.lock);
>  
> @@ -156,7 +157,9 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
>  		num_flip_ctrls = 0;
>  	}
>  
> -	vsp1_rwpf_init_ctrls(wpf, num_flip_ctrls);
> +	ret = vsp1_rwpf_init_ctrls(wpf, num_flip_ctrls);
> +	if (ret < 0)
> +		return ret;
>  
>  	if (num_flip_ctrls >= 1) {
>  		wpf->flip.ctrls.vflip =
> @@ -174,11 +177,8 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
>  		v4l2_ctrl_cluster(3, &wpf->flip.ctrls.vflip);
>  	}
>  
> -	if (wpf->ctrls.error) {
> -		dev_err(vsp1->dev, "wpf%u: failed to initialize controls\n",
> -			wpf->entity.index);
> +	if (wpf->ctrls.error)
>  		return wpf->ctrls.error;
> -	}
>  
>  	return 0;
>  }
> 
> -- 
> 2.48.1
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v7 5/5] media: vsp1: Add VSPX support

[kernel test robot]

Hi Jacopo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on f2151613e040973c868d28c8b00885dfab69eb75]

url:    https://github.com/intel-lab-lkp/linux/commits/Jacopo-Mondi/media-vsp1-Add-support-IIF-ISP-Interface/20250401-222806
base:   f2151613e040973c868d28c8b00885dfab69eb75
patch link:    https://lore.kernel.org/r/20250401-v4h-iif-v7-5-cc547c0bddd5%40ideasonboard.com
patch subject: [PATCH v7 5/5] media: vsp1: Add VSPX support
config: i386-buildonly-randconfig-004-20250402 (https://download.01.org/0day-ci/archive/20250402/202504020532.rdq3yOcN-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250402/202504020532.rdq3yOcN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504020532.rdq3yOcN-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/media/platform/renesas/vsp1/vsp1_vspx.c: In function 'vsp1_vspx_cleanup':
>> drivers/media/platform/renesas/vsp1/vsp1_vspx.c:599:36: warning: variable 'vspx_pipe' set but not used [-Wunused-but-set-variable]
     599 |         struct vsp1_vspx_pipeline *vspx_pipe;
         |                                    ^~~~~~~~~
--
>> drivers/media/platform/renesas/vsp1/vsp1_vspx.c:395: warning: Function parameter or struct member 'desc' not described in 'vsp1_isp_job_prepare'
>> drivers/media/platform/renesas/vsp1/vsp1_vspx.c:395: warning: expecting prototype for vsp1_vspx_job_prepare(). Prototype was for vsp1_isp_job_prepare() instead


vim +/vspx_pipe +599 drivers/media/platform/renesas/vsp1/vsp1_vspx.c

   596	
   597	void vsp1_vspx_cleanup(struct vsp1_device *vsp1)
   598	{
 > 599		struct vsp1_vspx_pipeline *vspx_pipe;

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v7 5/5] media: vsp1: Add VSPX support

[ALOK TIWARI]

Hi Jacopo,

On 01-04-2025 19:52, Jacopo Mondi wrote:
> Add support for VSPX, a specialized version of the VSP2 that
> transfer data to the ISP. The VSPX is composed by two RPF units
> to read data from external memory and an IIF instance that performs
> transfer towards the ISP.
> 
> The VSPX is supported through a newly introduced vsp1_vspx.c file that
> exposes two interfaces: vsp1_vspx interface, declared in vsp1_vspx.h
> for the vsp1 core to initialize and cleanup the VSPX, and a vsp1_isp
> interface, declared in include/media/vsp1.h for the ISP driver to
> control the VSPX operations.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---
>   drivers/media/platform/renesas/vsp1/Makefile    |   1 +
>   drivers/media/platform/renesas/vsp1/vsp1.h      |   1 +
>   drivers/media/platform/renesas/vsp1/vsp1_drv.c  |  13 +-
>   drivers/media/platform/renesas/vsp1/vsp1_regs.h |   1 +
>   drivers/media/platform/renesas/vsp1/vsp1_vspx.c | 604 ++++++++++++++++++++++++
>   drivers/media/platform/renesas/vsp1/vsp1_vspx.h |  86 ++++
>   include/media/vsp1.h                            |  73 +++
>   7 files changed, 778 insertions(+), 1 deletion(-)
> 

> +
> +/**
> + * vsp1_isp_alloc_buffers - Allocate buffers in the VSPX address space
> + *
> + * Allocate buffers that will be later accessed by the VSPX.
> + *
> + * @dev: The VSP1 struct device
> + * @size: The size of the buffer to be allocated by the VSPX
> + * @buffer_desc: The allocated buffer description, will be filled with the
> + *		 buffer CPU-mapped address and the bus address
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> +			   struct vsp1_isp_buffer_desc *buffer_desc)
> +{
> +	struct device *bus_master = vsp1_isp_get_bus_master(dev);
> +
> +	if (IS_ERR_OR_NULL(bus_master))
> +		return -ENODEV;
> +
> +	buffer_desc->cpu_addr = dma_alloc_coherent(bus_master, size,
> +						   &buffer_desc->dma_addr,
> +						   GFP_KERNEL);
> +	if (IS_ERR_OR_NULL(buffer_desc->cpu_addr))
> +		return -EINVAL;
why use -EINVAL instead -ENOMEM

Since dma_alloc_coherent() never returns error-encoded pointers
IS_ERR(ptr) check is not meaningless here ?
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_alloc_buffers);
> +
...

> + * struct vsp1_vspx_frame_end - VSPX frame end callback data
> + *
> + * @vspx_frame_end: Frame end callback. Called after a transfer job has been
> + *		    completed. If the job includes both a ConfigDMA and a
> + *		    RAW image, the callback is called after both have been
> + *		    transferred
> + * @frame_end_data: Frame end callback data, passed to vspx_frame_end
> + */
> +struct vsp1_vspx_frame_end {
> +	void (*vspx_frame_end)(void *data);
> +	void *frame_end_data;
> +};
> +
> +int vsp1_isp_init(struct device *dev);
> +struct device *vsp1_isp_get_bus_master(struct device *dev);
> +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> +			   struct vsp1_isp_buffer_desc *buffer_desc);
> +int vsp1_isp_configure(struct device *dev,
> +		       const struct v4l2_pix_format_mplane *fmt);
> +int vsp1_isp_start_streaming(struct device *dev,
> +			     struct vsp1_vspx_frame_end *frame_end,
> +			     bool enable);
> +int vsp1_isp_job_prepare(struct device *dev,
> +			 const struct vsp1_isp_job_desc *desc);
> +void vsp1_isp_job_run(struct device *dev);
> +
>   #endif /* __MEDIA_VSP1_H__ */
> 

Thanks,
Alok

Re: [PATCH v7 5/5] media: vsp1: Add VSPX support

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Apr 01, 2025 at 04:22:05PM +0200, Jacopo Mondi wrote:
> Add support for VSPX, a specialized version of the VSP2 that
> transfer data to the ISP. The VSPX is composed by two RPF units

It seems you forgot to take comments from v2 into account.

> to read data from external memory and an IIF instance that performs
> transfer towards the ISP.
> 
> The VSPX is supported through a newly introduced vsp1_vspx.c file that
> exposes two interfaces: vsp1_vspx interface, declared in vsp1_vspx.h
> for the vsp1 core to initialize and cleanup the VSPX, and a vsp1_isp
> interface, declared in include/media/vsp1.h for the ISP driver to
> control the VSPX operations.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/vsp1/Makefile    |   1 +
>  drivers/media/platform/renesas/vsp1/vsp1.h      |   1 +
>  drivers/media/platform/renesas/vsp1/vsp1_drv.c  |  13 +-
>  drivers/media/platform/renesas/vsp1/vsp1_regs.h |   1 +
>  drivers/media/platform/renesas/vsp1/vsp1_vspx.c | 604 ++++++++++++++++++++++++
>  drivers/media/platform/renesas/vsp1/vsp1_vspx.h |  86 ++++
>  include/media/vsp1.h                            |  73 +++
>  7 files changed, 778 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/Makefile b/drivers/media/platform/renesas/vsp1/Makefile
> index de8c802e1d1a..2057c8f7be47 100644
> --- a/drivers/media/platform/renesas/vsp1/Makefile
> +++ b/drivers/media/platform/renesas/vsp1/Makefile
> @@ -6,5 +6,6 @@ vsp1-y					+= vsp1_clu.o vsp1_hsit.o vsp1_lut.o
>  vsp1-y					+= vsp1_brx.o vsp1_sru.o vsp1_uds.o
>  vsp1-y					+= vsp1_hgo.o vsp1_hgt.o vsp1_histo.o
>  vsp1-y					+= vsp1_iif.o vsp1_lif.o vsp1_uif.o
> +vsp1-y					+= vsp1_vspx.o
>  
>  obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1.o
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1.h b/drivers/media/platform/renesas/vsp1/vsp1.h
> index 263024639dd2..1872e403f26b 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1.h
> @@ -110,6 +110,7 @@ struct vsp1_device {
>  	struct media_entity_operations media_ops;
>  
>  	struct vsp1_drm *drm;
> +	struct vsp1_vspx *vspx;
>  };
>  
>  int vsp1_device_get(struct vsp1_device *vsp1);
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> index d13e9b31aa7c..e338ab8af292 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> @@ -38,6 +38,7 @@
>  #include "vsp1_uds.h"
>  #include "vsp1_uif.h"
>  #include "vsp1_video.h"
> +#include "vsp1_vspx.h"
>  
>  /* -----------------------------------------------------------------------------
>   * Interrupt Handling
> @@ -488,7 +489,10 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
>  
>  		ret = media_device_register(mdev);
>  	} else {
> -		ret = vsp1_drm_init(vsp1);
> +		if (vsp1->info->version == VI6_IP_VERSION_MODEL_VSPX_GEN4)
> +			ret = vsp1_vspx_init(vsp1);
> +		else
> +			ret = vsp1_drm_init(vsp1);
>  	}
>  
>  done:
> @@ -846,6 +850,13 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
>  		.uif_count = 2,
>  		.wpf_count = 1,
>  		.num_bru_inputs = 5,
> +	}, {
> +		.version = VI6_IP_VERSION_MODEL_VSPX_GEN4,
> +		.model = "VSP2-X",
> +		.gen = 4,
> +		.features = VSP1_HAS_IIF,
> +		.rpf_count = 2,
> +		.wpf_count = 1,
>  	},
>  };
>  
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_regs.h b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> index 86e47c2d991f..10cfbcd1b6e0 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> @@ -799,6 +799,7 @@
>  #define VI6_IP_VERSION_MODEL_VSPDL_GEN3	(0x19 << 8)
>  #define VI6_IP_VERSION_MODEL_VSPBS_GEN3	(0x1a << 8)
>  #define VI6_IP_VERSION_MODEL_VSPD_GEN4	(0x1c << 8)
> +#define VI6_IP_VERSION_MODEL_VSPX_GEN4	(0x1d << 8)
>  /* RZ/G2L SoCs have no version register, So use 0x80 as the model version */
>  #define VI6_IP_VERSION_MODEL_VSPD_RZG2L	(0x80 << 8)
>  
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_vspx.c b/drivers/media/platform/renesas/vsp1/vsp1_vspx.c
> new file mode 100644
> index 000000000000..db9707f2b532
> --- /dev/null
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_vspx.c
> @@ -0,0 +1,604 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * vsp1_vspx.c  --  R-Car Gen 4 VSPX
> + *
> + * Copyright (C) 2025 Ideas On Board Oy
> + * Copyright (C) 2025 Renesas Electronics Corporation
> + */
> +
> +#include "vsp1_vspx.h"
> +
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/slab.h>
> +
> +#include <media/media-entity.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/vsp1.h>
> +
> +#include "vsp1.h"
> +#include "vsp1_dl.h"
> +#include "vsp1_iif.h"
> +#include "vsp1_pipe.h"
> +#include "vsp1_rwpf.h"
> +
> +static const struct v4l2_pix_format_mplane vspx_default_fmt = {
> +	.width = 1920,
> +	.height = 1080,
> +	.pixelformat = V4L2_PIX_FMT_SRGGB8,
> +	.field = V4L2_FIELD_NONE,
> +	.num_planes = 1,
> +	.plane_fmt = {
> +		[0] = {
> +			.sizeimage = 1920 * 1080,
> +			.bytesperline = 1920,
> +		},
> +	},
> +};
> +
> +/*
> + * Apply the given width, height and fourcc to the subdevice inside the
> + * VSP1 entity.
> + */
> +static int vsp1_vspx_rwpf_set_subdev_fmt(struct vsp1_device *vsp1,
> +					 struct vsp1_rwpf *rwpf,
> +					 u32 isp_fourcc,
> +					 unsigned int width,
> +					 unsigned int height)
> +{
> +	struct vsp1_entity *ent = &rwpf->entity;
> +	const struct vsp1_format_info *fmtinfo;
> +	struct v4l2_subdev_format format = {};
> +	u32 vspx_fourcc;
> +	int ret;
> +
> +	switch (isp_fourcc) {
> +	case V4L2_PIX_FMT_GREY:
> +		/* 8 bit RAW Bayer */
> +		vspx_fourcc = V4L2_PIX_FMT_RGB332;
> +		break;
> +	case V4L2_PIX_FMT_Y10:
> +	case V4L2_PIX_FMT_Y12:
> +	case V4L2_PIX_FMT_Y16:
> +		/* 10, 12 and 16 bit RAW Bayer */
> +		vspx_fourcc = V4L2_PIX_FMT_RGB565;
> +		break;
> +	case V4L2_PIX_FMT_XBGR32:

As it's metadata, how about using V4L2_META_FMT_GENERIC_8 instead ?
Usage of this format is internal to this file, so the 4CC value doesn't
matter much (as long as it's different from the other image formats),
but using a META format would make the code slightly clearer I think.

> +		/* ConfigDMA */
> +		vspx_fourcc = V4L2_PIX_FMT_XBGR32;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	fmtinfo = vsp1_get_format_info(vsp1, vspx_fourcc);
> +	if (!fmtinfo) {
> +		dev_dbg(vsp1->dev, "Unsupported pixel format %08x\n",
> +			vspx_fourcc);
> +		return -EINVAL;
> +	}

You could drop the check, as vspx_fourcc is guaranteed to be supported.

> +
> +	rwpf->fmtinfo = fmtinfo;
> +
> +	format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +	format.pad = RWPF_PAD_SINK;
> +	format.format.width = width;
> +	format.format.height = height;
> +	format.format.field = V4L2_FIELD_NONE;
> +	format.format.code = fmtinfo->mbus;
> +
> +	ret = v4l2_subdev_call(&ent->subdev, pad, set_fmt, NULL, &format);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/*
> + * Configure RPF0 for ConfigDMA or RAW image transfer.
> + */
> +static int vsp1_vspx_rpf0_configure(struct vsp1_device *vsp1,
> +				    dma_addr_t addr, u32 isp_fourcc,
> +				    unsigned int width, unsigned int height,
> +				    unsigned int stride,
> +				    unsigned int iif_sink_pad,
> +				    struct vsp1_dl_list *dl,
> +				    struct vsp1_dl_body *dlb)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> +	struct vsp1_rwpf *rpf0 = pipe->inputs[0];
> +	int ret;
> +
> +	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, rpf0, isp_fourcc, width,
> +					    height);
> +	if (ret)
> +		return ret;
> +
> +	rpf0->format.plane_fmt[0].bytesperline = stride;
> +
> +	/*
> +	 * Connect RPF0 to the IIF sink pad corresponding to the config or image
> +	 * path.
> +	 */
> +	rpf0->entity.sink_pad = iif_sink_pad;
> +
> +	pipe->part_table[0].rpf[0].width = width;
> +	pipe->part_table[0].rpf[0].height = height;
> +
> +	rpf0->mem.addr[0] = addr;
> +	rpf0->mem.addr[1] = 0;
> +	rpf0->mem.addr[2] = 0;
> +
> +	vsp1_entity_route_setup(&rpf0->entity, pipe, dlb);
> +	vsp1_entity_configure_stream(&rpf0->entity, rpf0->entity.state, pipe,
> +				     dl, dlb);
> +	vsp1_entity_configure_partition(&rpf0->entity, pipe,
> +					&pipe->part_table[0], dl, dlb);
> +
> +	return 0;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Interrupt handling
> + */

Missing blank line.

> +static void vsp1_vspx_pipeline_frame_end(struct vsp1_pipeline *pipe,
> +					 unsigned int completion)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe = to_vsp1_vspx_pipeline(pipe);
> +	struct vsp1_vspx *vspx = to_vsp1_vspx(vspx_pipe);
> +
> +	if (vspx_pipe->vspx_frame_end)
> +		vspx_pipe->vspx_frame_end(vspx_pipe->frame_end_data);
> +
> +	/*
> +	 * Set the pipeline state to stopped to ensure the next call to
> +	 * vsp1_pipeline_run() actually starts the VSPX.
> +	 */
> +	scoped_guard(spinlock_irqsave, &pipe->irqlock) {

As this function is called from an interrupt handler only, you could use
a plain spinlock.

> +		pipe->state = VSP1_PIPELINE_STOPPED;
> +	}
> +
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {

Here too.

> +		vspx_pipe->processing = false;
> +	}
> +
> +	/* Try schedule a new job from the queue. */
> +	vsp1_isp_job_run(vspx->vsp1->dev);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * ISP Driver API (include/media/vsp1.h)
> + */
> +
> +/**
> + * vsp1_isp_init() - Initialize the VSPX
> + *
> + * @dev: The VSP1 struct device
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_init(struct device *dev)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +
> +	if (!vsp1)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_init);
> +
> +/**
> + * vsp1_isp_get_bus_master - Get VSPX bus master
> + *
> + * The VSPX access memory through an FCPX instance. When allocating memory

s/access/accesses/

> + * buffers that will have to be accessed by the VSPX the 'struct device' of
> + * the FCPX should be used. Use this function to get a reference to it.
> + *
> + * @dev: The VSP1 struct device
> + *
> + * Return: a pointer to the bus master's device
> + */
> +struct device *vsp1_isp_get_bus_master(struct device *dev)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +
> +	if (!vsp1)
> +		return ERR_PTR(-ENODEV);
> +
> +	return vsp1->bus_master;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_get_bus_master);
> +
> +/**
> + * vsp1_isp_alloc_buffers - Allocate buffers in the VSPX address space

s/vsp1_isp_alloc_buffers/vsp1_isp_alloc_buffer/

and s/buffers/buffer/ in the documentation.

> + *
> + * Allocate buffers that will be later accessed by the VSPX.
> + *
> + * @dev: The VSP1 struct device
> + * @size: The size of the buffer to be allocated by the VSPX
> + * @buffer_desc: The allocated buffer description, will be filled with the

s/description/descriptor/

> + *		 buffer CPU-mapped address and the bus address
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> +			   struct vsp1_isp_buffer_desc *buffer_desc)
> +{
> +	struct device *bus_master = vsp1_isp_get_bus_master(dev);
> +
> +	if (IS_ERR_OR_NULL(bus_master))
> +		return -ENODEV;
> +
> +	buffer_desc->cpu_addr = dma_alloc_coherent(bus_master, size,
> +						   &buffer_desc->dma_addr,
> +						   GFP_KERNEL);
> +	if (IS_ERR_OR_NULL(buffer_desc->cpu_addr))
> +		return -EINVAL;

As commented by Alok, this should be

	if (!buffer_desc->cpu_addr)
		return -ENOMEM;

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_alloc_buffers);

Where is the buffer freed ?

> +
> +/**
> + * vsp1_isp_configure - Configure the VSPX with the RAW image format
> + *
> + * Apply to the VSPX RPF/WPF the size of the RAW image that will be transferred
> + * to the ISP.
> + *
> + * @dev: The VSP1 struct device
> + * @fmt: The RAW image format description
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_configure(struct device *dev,
> +		       const struct v4l2_pix_format_mplane *fmt)

I think this function should be dropped, the format should be passed
along with each job.

> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_vspx_pipeline *vspx_pipe;
> +	struct vsp1_pipeline *pipe;
> +	int ret;
> +
> +	vspx_pipe = &vsp1->vspx->pipe;
> +	pipe = &vspx_pipe->pipe;
> +
> +	/*
> +	 * Apply the same format to the RPF0 and WPF0 so that the partition
> +	 * calculation results in a single partition.
> +	 */
> +	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->inputs[0],
> +					    fmt->pixelformat, fmt->width,
> +					    fmt->height);
> +	if (ret)
> +		return ret;
> +
> +	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->output, fmt->pixelformat,
> +					    fmt->width, fmt->height);
> +	if (ret)
> +		return ret;
> +
> +	vsp1_pipeline_calculate_partition(pipe, &pipe->part_table[0],
> +					  fmt->width, 0);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_configure);
> +
> +static void vsp1_vspx_release_jobs(struct vsp1_device *vsp1)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_vspx_job *job, *tmp;
> +
> +	guard(spinlock_irqsave)(&vspx_pipe->jobs_lock);
> +
> +	list_for_each_entry_safe(job, tmp, &vspx_pipe->jobs, job_queue) {
> +		list_del(&job->job_queue);
> +		vsp1_dl_list_put(job->dl);
> +		kfree(job);
> +	}
> +}
> +
> +/**
> + * vsp1_isp_start_streaming - Start processing VSPX jobs
> + *
> + * Start the VSPX and prepare for accepting buffer transfer job requests.
> + *
> + * @dev: The VSP1 struct device
> + * @frame_end: The frame end callback description
> + * @enable: The enable/disable streaming flag
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_start_streaming(struct device *dev,
> +			     struct vsp1_vspx_frame_end *frame_end,
> +			     bool enable)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> +	u32 value;
> +	int ret;
> +
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> +		if (vspx_pipe->enabled == enable)
> +			return 0;
> +
> +		vspx_pipe->enabled = enable;
> +	}
> +
> +	if (!enable) {
> +		pipe->state = VSP1_PIPELINE_STOPPED;
> +		vsp1_pipeline_stop(pipe);
> +		vsp1_vspx_release_jobs(vsp1);
> +		vspx_pipe->processing = false;
> +		vspx_pipe->vspx_frame_end = NULL;
> +		vsp1_dlm_reset(pipe->output->dlm);
> +		vsp1_device_put(vsp1);
> +		return 0;
> +	}
> +
> +	if (!frame_end) {
> +		ret = -EINVAL;
> +		goto error_stop_pipe;
> +	}
> +
> +	vspx_pipe->vspx_frame_end = frame_end->vspx_frame_end;
> +	vspx_pipe->frame_end_data = frame_end->frame_end_data;
> +
> +	/* Make sure VSPX is not active. */
> +	value = vsp1_read(vsp1, VI6_CMD(0));
> +	if (value & VI6_CMD_STRCMD) {
> +		dev_err(vsp1->dev,
> +			"%s: Starting of WPF0 already reserved\n", __func__);
> +		ret = -EBUSY;
> +		goto error_stop_pipe;
> +	}
> +
> +	value = vsp1_read(vsp1, VI6_STATUS);
> +	if (value & VI6_STATUS_SYS_ACT(0)) {
> +		dev_err(vsp1->dev,
> +			"%s: WPF0 has not entered idle state\n", __func__);
> +		ret = -EBUSY;
> +		goto error_stop_pipe;
> +	}
> +
> +	/* Enable the VSP1 and prepare for streaming. */
> +	vsp1_pipeline_dump(pipe, "VSPX job");
> +
> +	ret = vsp1_device_get(vsp1);
> +	if (ret < 0)
> +		goto error_stop_pipe;
> +
> +	return 0;
> +
> +error_stop_pipe:
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> +		vspx_pipe->enabled = false;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_start_streaming);
> +
> +/**
> + * vsp1_vspx_job_prepare - Prepare a display list with the content of the buffer
> + *
> + * @dev: The VSP1 struct device
> + * @job: The job description
> + *
> + * Return: %0 on success or a negative error code on failure
> + */
> +int vsp1_isp_job_prepare(struct device *dev,
> +			 const struct vsp1_isp_job_desc *desc)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> +	const struct v4l2_pix_format_mplane *pix_mp;
> +	struct vsp1_dl_list *second_dl = NULL;
> +	struct vsp1_vspx_job *job;
> +	struct vsp1_dl_body *dlb;
> +	struct vsp1_dl_list *dl;
> +	int ret;
> +
> +	/*
> +	 * Populate a display list and append it to the jobs queue.
> +	 * Memory is released when the job is consumed.
> +	 */
> +	job = kmalloc(sizeof(*job), GFP_KERNEL);
> +	if (!job)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Transfer the buffers described in the job: (optional) ConfigDMA and
> +	 * RAW image.
> +	 */
> +
> +	job->dl = vsp1_dl_list_get(pipe->output->dlm);
> +	if (!job->dl) {
> +		ret = -ENOMEM;
> +		goto error_free_job;
> +	}
> +
> +	dl = job->dl;
> +	dlb = vsp1_dl_list_get_body0(dl);
> +
> +	/* Disable RPF1. */
> +	vsp1_dl_body_write(dlb, vsp1->rpf[1]->entity.route->reg,
> +			   VI6_DPR_NODE_UNUSED);
> +
> +	/* Configure IIF routing and enable IIF function */
> +	vsp1_entity_route_setup(pipe->iif, pipe, dlb);
> +	vsp1_entity_configure_stream(pipe->iif, pipe->iif->state, pipe,
> +				     dl, dlb);
> +
> +	/* Configure WPF0 to enable RPF0 as source*/
> +	vsp1_entity_route_setup(&pipe->output->entity, pipe, dlb);
> +	vsp1_entity_configure_stream(&pipe->output->entity,
> +				     pipe->output->entity.state, pipe,
> +				     dl, dlb);
> +
> +	if (desc->config.pairs) {
> +		/*
> +		 * Configure RPF0 for config data. Transfer the number of
> +		 * configuration pairs plus 2 words for the header.
> +		 */
> +		ret = vsp1_vspx_rpf0_configure(vsp1, desc->config.mem,
> +					       V4L2_PIX_FMT_XBGR32,
> +					       desc->config.pairs * 2 + 2, 1,
> +					       desc->config.pairs * 2 + 2,
> +					       VSPX_IIF_SINK_PAD_CONFIG,
> +					       dl, dlb);
> +		if (ret)
> +			goto error_put_dl;
> +
> +		second_dl = vsp1_dl_list_get(pipe->output->dlm);
> +		if (!second_dl) {
> +			ret = -ENOMEM;
> +			goto error_put_dl;
> +		}
> +
> +		dl = second_dl;
> +		dlb = vsp1_dl_list_get_body0(dl);
> +	}
> +
> +	/* Configure RPF0 for RAW image transfer. */
> +	pix_mp = &desc->img.fmt.fmt.pix_mp;
> +	ret = vsp1_vspx_rpf0_configure(vsp1, desc->img.mem,
> +				       pix_mp->pixelformat,
> +				       pix_mp->width, pix_mp->height,
> +				       pix_mp->plane_fmt[0].bytesperline,
> +				       VSPX_IIF_SINK_PAD_IMG, dl, dlb);
> +	if (ret)
> +		goto error_put_dl;
> +
> +	if (second_dl)
> +		vsp1_dl_list_add_chain(job->dl, second_dl);
> +
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->jobs_lock) {
> +		list_add_tail(&job->job_queue, &vspx_pipe->jobs);
> +	}
> +
> +	return 0;
> +
> +error_put_dl:
> +	if (second_dl)
> +		vsp1_dl_list_put(second_dl);
> +	vsp1_dl_list_put(job->dl);
> +error_free_job:
> +	kfree(job);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_job_prepare);
> +
> +/**
> + * vsp1_isp_job_run - Run a buffer transfer on behalf of the ISP
> + *
> + * @dev: The VSP1 struct device
> + */
> +void vsp1_isp_job_run(struct device *dev)
> +{
> +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> +	struct vsp1_vspx_job *job;
> +
> +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> +
> +		if (vspx_pipe->processing)
> +			return;
> +
> +		/* Extract one job, if available, from the jobs list. */
> +		scoped_guard(spinlock_irqsave, &vspx_pipe->jobs_lock) {
> +			job = list_first_entry_or_null(&vspx_pipe->jobs,
> +						       struct vsp1_vspx_job,
> +						       job_queue);
> +			if (!job)
> +				return;
> +
> +			list_del(&job->job_queue);
> +		}
> +
> +		vspx_pipe->processing = true;
> +		vsp1_dl_list_commit(job->dl, 0);
> +		kfree(job);
> +	}
> +
> +	/* Trigger VSPX processing by setting VI6_CMD[STRCMD]. */
> +	scoped_guard(spinlock_irqsave, &pipe->irqlock) {
> +		vsp1_pipeline_run(pipe);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(vsp1_isp_job_run);
> +
> +/* -----------------------------------------------------------------------------
> + * Initialization and cleanup
> + */
> +
> +int vsp1_vspx_init(struct vsp1_device *vsp1)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe;
> +	struct vsp1_pipeline *pipe;
> +
> +	vsp1->vspx = devm_kzalloc(vsp1->dev, sizeof(*vsp1->vspx), GFP_KERNEL);
> +	if (!vsp1->vspx)
> +		return -ENOMEM;
> +
> +	vsp1->vspx->vsp1 = vsp1;
> +
> +	vspx_pipe = &vsp1->vspx->pipe;
> +	vspx_pipe->processing = false;
> +	vspx_pipe->enabled = false;
> +
> +	pipe = &vspx_pipe->pipe;
> +
> +	vsp1_pipeline_init(pipe);
> +
> +	pipe->partitions = 1;
> +	pipe->part_table = &vspx_pipe->partition;
> +	pipe->interlaced = false;
> +	pipe->frame_end = vsp1_vspx_pipeline_frame_end;
> +
> +	INIT_LIST_HEAD(&vspx_pipe->jobs);
> +	spin_lock_init(&vspx_pipe->vspx_lock);
> +	spin_lock_init(&vspx_pipe->jobs_lock);
> +
> +	/*
> +	 * Initialize RPF0 as inputs for VSPX and use it unconditionally for

s/inputs/input/

> +	 * now.
> +	 */
> +	pipe->inputs[0] = vsp1->rpf[0];
> +	pipe->inputs[0]->entity.pipe = pipe;
> +	pipe->inputs[0]->entity.sink = &vsp1->iif->entity;
> +	vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->inputs[0],
> +				      vspx_default_fmt.pixelformat,
> +				      vspx_default_fmt.width,
> +				      vspx_default_fmt.height);
> +	list_add(&pipe->inputs[0]->entity.list_pipe, &pipe->entities);

My comment regarding switching from list_add_tail() to list_add() was
based on the fact you were initializing the entities in reverse order
(first WPF, then IIF, the RPF). Now that you swapped that,
list_add_tail() is correct.

> +
> +	pipe->iif = &vsp1->iif->entity;
> +	pipe->iif->pipe = pipe;
> +	pipe->iif->sink = &vsp1->wpf[0]->entity;
> +	list_add(&pipe->iif->list_pipe, &pipe->entities);
> +
> +	pipe->output = vsp1->wpf[0];
> +	pipe->output->entity.pipe = pipe;
> +	vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->output,
> +				      vspx_default_fmt.pixelformat,
> +				      vspx_default_fmt.width,
> +				      vspx_default_fmt.height);
> +	list_add(&pipe->output->entity.list_pipe, &pipe->entities);
> +
> +	return 0;
> +}
> +
> +void vsp1_vspx_cleanup(struct vsp1_device *vsp1)
> +{
> +	struct vsp1_vspx_pipeline *vspx_pipe;
> +
> +	vspx_pipe = &vsp1->vspx->pipe;

Unused variable.

> +
> +	vsp1_vspx_release_jobs(vsp1);
> +}
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_vspx.h b/drivers/media/platform/renesas/vsp1/vsp1_vspx.h
> new file mode 100644
> index 000000000000..2f68f043075e
> --- /dev/null
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_vspx.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * vsp1_vspx.h  --  R-Car Gen 4 VSPX
> + *
> + * Copyright (C) 2025 Ideas On Board Oy
> + * Copyright (C) 2025 Renesas Electronics Corporation
> + */
> +#ifndef __VSP1_VSPX_H__
> +#define __VSP1_VSPX_H__
> +
> +#include <linux/container_of.h>
> +#include <linux/list.h>
> +#include <linux/spinlock.h>
> +
> +#include <media/vsp1.h>

I don't think this header is needed.

> +
> +#include "vsp1.h"
> +#include "vsp1_pipe.h"
> +
> +/* Pixel format for ConfigDMA buffers. */
> +#define V4L2_META_FMT_RCAR_V4H	v4l2_fourcc('R', 'C', 'A', '4')

Is this needed ?

> +
> +/*
> + * struct vsp1_vspx_pipeline - VSPX pipeline
> + * @pipe: the VSP1 pipeline
> + * @partition: the pre-calculated partition used by the pipeline
> + * @vspx_lock: protect access to the VSPX configuration
> + * @processing: VSPX busy flag
> + * @jobs_lock: protect the jobs queue
> + * @jobs: jobs queue
> + * @vspx_frame_end: frame end callback
> + * @frame_end_data: data for the frame end callback
> + */
> +struct vsp1_vspx_pipeline {
> +	struct vsp1_pipeline pipe;
> +	struct vsp1_partition partition;
> +
> +	/* Protects the pipeline configuration */
> +	spinlock_t vspx_lock;
> +	bool processing;
> +	bool enabled;
> +
> +	/* Protects the jobs list */
> +	spinlock_t jobs_lock;
> +	struct list_head jobs;
> +
> +	void (*vspx_frame_end)(void *frame_end_data);
> +	void *frame_end_data;
> +};
> +
> +static inline struct vsp1_vspx_pipeline *
> +to_vsp1_vspx_pipeline(struct vsp1_pipeline *pipe)
> +{
> +	return container_of(pipe, struct vsp1_vspx_pipeline, pipe);
> +}
> +
> +/*
> + * struct vsp1_vspx_job - VSPX transfer job
> + * @dl: Display list populated by vsp1_isp_job_prepare
> + * @job_queue: List handle
> + */
> +struct vsp1_vspx_job {
> +	struct vsp1_dl_list *dl;
> +	struct list_head job_queue;
> +};
> +
> +/*
> + * struct vsp1_vspx - VSPX device
> + * @vsp1: the VSP1 device
> + * @pipe: the VSPX pipeline
> + */
> +struct vsp1_vspx {
> +	struct vsp1_device *vsp1;
> +	struct vsp1_vspx_pipeline pipe;
> +};
> +
> +static inline struct vsp1_vspx *
> +to_vsp1_vspx(struct vsp1_vspx_pipeline *vspx_pipe)
> +{
> +	return container_of(vspx_pipe, struct vsp1_vspx, pipe);
> +}
> +
> +int vsp1_vspx_init(struct vsp1_device *vsp1);
> +void vsp1_vspx_cleanup(struct vsp1_device *vsp1);
> +
> +#endif /* __VSP1_VSPX_H__ */
> diff --git a/include/media/vsp1.h b/include/media/vsp1.h
> index 48f4a5023d81..68e4d9891dff 100644
> --- a/include/media/vsp1.h
> +++ b/include/media/vsp1.h
> @@ -9,12 +9,17 @@
>  #ifndef __MEDIA_VSP1_H__
>  #define __MEDIA_VSP1_H__
>  
> +#include <linux/list.h>

Not noeeded anymore.

>  #include <linux/scatterlist.h>
>  #include <linux/types.h>
>  #include <linux/videodev2.h>
>  
>  struct device;
>  
> +/*******************************************************************************

/* -----------------------------------------------------------------------------

> + * VSP1 DU interface
> + */
> +
>  int vsp1_du_init(struct device *dev);
>  
>  #define VSP1_DU_STATUS_COMPLETE		BIT(0)
> @@ -117,4 +122,72 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
>  int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt);
>  void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt);
>  
> +/*******************************************************************************

Same here.

> + * VSP1 ISP interface
> + */
> +
> +/**
> + * struct vsp1_isp_buffer_desc - Describe a buffer allocated by VSPX
> + *
> + * @cpu_addr: CPU-mapped address of a buffer allocated by VSPX
> + * @dma_addr: bus address of a buffer allocated by VSPX
> + */
> +struct vsp1_isp_buffer_desc {
> +	void *cpu_addr;
> +	dma_addr_t dma_addr;
> +};
> +
> +/**
> + * struct vsp1_isp_job_desc - Describe a VSPX buffer transfer request
> + *
> + * Describe a transfer request for the VSPX to perform on behalf of the ISP.
> + * The transfer job descriptor contains an optional ConfigDMA buffer and
> + * one RAW image buffer. Set config.pairs to 0 if no ConfigDMA buffer should
> + * be transferred.
> + *
> + * @config: ConfigDMA buffer
> + * @config.pairs: number of reg-value pairs in the ConfigDMA buffer
> + * @config.mem: bus address of the ConfigDMA buffer
> + * @img: RAW image buffer
> + * @img.fmt: RAW image format
> + * @img.mem: bus address of the RAW image buffer
> + */
> +struct vsp1_isp_job_desc {
> +	struct {
> +		unsigned int pairs;
> +		dma_addr_t mem;
> +	} config;
> +	struct {
> +		struct v4l2_format fmt;
> +		dma_addr_t mem;
> +	} img;
> +};
> +
> +/**
> + * struct vsp1_vspx_frame_end - VSPX frame end callback data
> + *
> + * @vspx_frame_end: Frame end callback. Called after a transfer job has been
> + *		    completed. If the job includes both a ConfigDMA and a
> + *		    RAW image, the callback is called after both have been
> + *		    transferred
> + * @frame_end_data: Frame end callback data, passed to vspx_frame_end
> + */
> +struct vsp1_vspx_frame_end {
> +	void (*vspx_frame_end)(void *data);
> +	void *frame_end_data;
> +};
> +
> +int vsp1_isp_init(struct device *dev);
> +struct device *vsp1_isp_get_bus_master(struct device *dev);
> +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> +			   struct vsp1_isp_buffer_desc *buffer_desc);
> +int vsp1_isp_configure(struct device *dev,
> +		       const struct v4l2_pix_format_mplane *fmt);
> +int vsp1_isp_start_streaming(struct device *dev,
> +			     struct vsp1_vspx_frame_end *frame_end,
> +			     bool enable);
> +int vsp1_isp_job_prepare(struct device *dev,
> +			 const struct vsp1_isp_job_desc *desc);
> +void vsp1_isp_job_run(struct device *dev);
> +
>  #endif /* __MEDIA_VSP1_H__ */
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v7 5/5] media: vsp1: Add VSPX support

[Laurent Pinchart]

Hi Niklas,

On Tue, Apr 01, 2025 at 10:07:05PM +0200, Niklas Söderlund wrote:
> On 2025-04-01 16:22:05 +0200, Jacopo Mondi wrote:
> > Add support for VSPX, a specialized version of the VSP2 that
> > transfer data to the ISP. The VSPX is composed by two RPF units
> > to read data from external memory and an IIF instance that performs
> > transfer towards the ISP.
> > 
> > The VSPX is supported through a newly introduced vsp1_vspx.c file that
> > exposes two interfaces: vsp1_vspx interface, declared in vsp1_vspx.h
> > for the vsp1 core to initialize and cleanup the VSPX, and a vsp1_isp
> > interface, declared in include/media/vsp1.h for the ISP driver to
> > control the VSPX operations.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> > ---
> >  drivers/media/platform/renesas/vsp1/Makefile    |   1 +
> >  drivers/media/platform/renesas/vsp1/vsp1.h      |   1 +
> >  drivers/media/platform/renesas/vsp1/vsp1_drv.c  |  13 +-
> >  drivers/media/platform/renesas/vsp1/vsp1_regs.h |   1 +
> >  drivers/media/platform/renesas/vsp1/vsp1_vspx.c | 604 ++++++++++++++++++++++++
> >  drivers/media/platform/renesas/vsp1/vsp1_vspx.h |  86 ++++
> >  include/media/vsp1.h                            |  73 +++
> >  7 files changed, 778 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/renesas/vsp1/Makefile b/drivers/media/platform/renesas/vsp1/Makefile
> > index de8c802e1d1a..2057c8f7be47 100644
> > --- a/drivers/media/platform/renesas/vsp1/Makefile
> > +++ b/drivers/media/platform/renesas/vsp1/Makefile
> > @@ -6,5 +6,6 @@ vsp1-y					+= vsp1_clu.o vsp1_hsit.o vsp1_lut.o
> >  vsp1-y					+= vsp1_brx.o vsp1_sru.o vsp1_uds.o
> >  vsp1-y					+= vsp1_hgo.o vsp1_hgt.o vsp1_histo.o
> >  vsp1-y					+= vsp1_iif.o vsp1_lif.o vsp1_uif.o
> > +vsp1-y					+= vsp1_vspx.o
> >  
> >  obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1.o
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1.h b/drivers/media/platform/renesas/vsp1/vsp1.h
> > index 263024639dd2..1872e403f26b 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1.h
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1.h
> > @@ -110,6 +110,7 @@ struct vsp1_device {
> >  	struct media_entity_operations media_ops;
> >  
> >  	struct vsp1_drm *drm;
> > +	struct vsp1_vspx *vspx;
> >  };
> >  
> >  int vsp1_device_get(struct vsp1_device *vsp1);
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> > index d13e9b31aa7c..e338ab8af292 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> > @@ -38,6 +38,7 @@
> >  #include "vsp1_uds.h"
> >  #include "vsp1_uif.h"
> >  #include "vsp1_video.h"
> > +#include "vsp1_vspx.h"
> >  
> >  /* -----------------------------------------------------------------------------
> >   * Interrupt Handling
> > @@ -488,7 +489,10 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
> >  
> >  		ret = media_device_register(mdev);
> >  	} else {
> > -		ret = vsp1_drm_init(vsp1);
> > +		if (vsp1->info->version == VI6_IP_VERSION_MODEL_VSPX_GEN4)
> > +			ret = vsp1_vspx_init(vsp1);
> > +		else
> > +			ret = vsp1_drm_init(vsp1);
> >  	}
> >  
> >  done:
> > @@ -846,6 +850,13 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
> >  		.uif_count = 2,
> >  		.wpf_count = 1,
> >  		.num_bru_inputs = 5,
> > +	}, {
> > +		.version = VI6_IP_VERSION_MODEL_VSPX_GEN4,
> > +		.model = "VSP2-X",
> > +		.gen = 4,
> > +		.features = VSP1_HAS_IIF,
> > +		.rpf_count = 2,
> > +		.wpf_count = 1,
> >  	},
> >  };
> >  
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_regs.h b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> > index 86e47c2d991f..10cfbcd1b6e0 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_regs.h
> > @@ -799,6 +799,7 @@
> >  #define VI6_IP_VERSION_MODEL_VSPDL_GEN3	(0x19 << 8)
> >  #define VI6_IP_VERSION_MODEL_VSPBS_GEN3	(0x1a << 8)
> >  #define VI6_IP_VERSION_MODEL_VSPD_GEN4	(0x1c << 8)
> > +#define VI6_IP_VERSION_MODEL_VSPX_GEN4	(0x1d << 8)
> >  /* RZ/G2L SoCs have no version register, So use 0x80 as the model version */
> >  #define VI6_IP_VERSION_MODEL_VSPD_RZG2L	(0x80 << 8)
> >  
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_vspx.c b/drivers/media/platform/renesas/vsp1/vsp1_vspx.c
> > new file mode 100644
> > index 000000000000..db9707f2b532
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_vspx.c
> > @@ -0,0 +1,604 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * vsp1_vspx.c  --  R-Car Gen 4 VSPX
> > + *
> > + * Copyright (C) 2025 Ideas On Board Oy
> > + * Copyright (C) 2025 Renesas Electronics Corporation
> > + */
> > +
> > +#include "vsp1_vspx.h"
> > +
> > +#include <linux/cleanup.h>
> > +#include <linux/device.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/slab.h>
> > +
> > +#include <media/media-entity.h>
> > +#include <media/v4l2-subdev.h>
> > +#include <media/vsp1.h>
> > +
> > +#include "vsp1.h"
> > +#include "vsp1_dl.h"
> > +#include "vsp1_iif.h"
> > +#include "vsp1_pipe.h"
> > +#include "vsp1_rwpf.h"
> > +
> > +static const struct v4l2_pix_format_mplane vspx_default_fmt = {
> > +	.width = 1920,
> > +	.height = 1080,
> > +	.pixelformat = V4L2_PIX_FMT_SRGGB8,
> > +	.field = V4L2_FIELD_NONE,
> > +	.num_planes = 1,
> > +	.plane_fmt = {
> > +		[0] = {
> > +			.sizeimage = 1920 * 1080,
> > +			.bytesperline = 1920,
> > +		},
> > +	},
> > +};
> > +
> > +/*
> > + * Apply the given width, height and fourcc to the subdevice inside the
> > + * VSP1 entity.
> > + */
> > +static int vsp1_vspx_rwpf_set_subdev_fmt(struct vsp1_device *vsp1,
> > +					 struct vsp1_rwpf *rwpf,
> > +					 u32 isp_fourcc,
> > +					 unsigned int width,
> > +					 unsigned int height)
> > +{
> > +	struct vsp1_entity *ent = &rwpf->entity;
> > +	const struct vsp1_format_info *fmtinfo;
> > +	struct v4l2_subdev_format format = {};
> > +	u32 vspx_fourcc;
> > +	int ret;
> > +
> > +	switch (isp_fourcc) {
> > +	case V4L2_PIX_FMT_GREY:
> > +		/* 8 bit RAW Bayer */
> > +		vspx_fourcc = V4L2_PIX_FMT_RGB332;
> > +		break;
> > +	case V4L2_PIX_FMT_Y10:
> > +	case V4L2_PIX_FMT_Y12:
> > +	case V4L2_PIX_FMT_Y16:
> > +		/* 10, 12 and 16 bit RAW Bayer */
> > +		vspx_fourcc = V4L2_PIX_FMT_RGB565;
> > +		break;
> > +	case V4L2_PIX_FMT_XBGR32:
> > +		/* ConfigDMA */
> > +		vspx_fourcc = V4L2_PIX_FMT_XBGR32;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	fmtinfo = vsp1_get_format_info(vsp1, vspx_fourcc);
> > +	if (!fmtinfo) {
> > +		dev_dbg(vsp1->dev, "Unsupported pixel format %08x\n",
> > +			vspx_fourcc);
> > +		return -EINVAL;
> > +	}
> > +
> > +	rwpf->fmtinfo = fmtinfo;
> > +
> > +	format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > +	format.pad = RWPF_PAD_SINK;
> > +	format.format.width = width;
> > +	format.format.height = height;
> > +	format.format.field = V4L2_FIELD_NONE;
> > +	format.format.code = fmtinfo->mbus;
> > +
> > +	ret = v4l2_subdev_call(&ent->subdev, pad, set_fmt, NULL, &format);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * Configure RPF0 for ConfigDMA or RAW image transfer.
> > + */
> > +static int vsp1_vspx_rpf0_configure(struct vsp1_device *vsp1,
> > +				    dma_addr_t addr, u32 isp_fourcc,
> > +				    unsigned int width, unsigned int height,
> > +				    unsigned int stride,
> > +				    unsigned int iif_sink_pad,
> > +				    struct vsp1_dl_list *dl,
> > +				    struct vsp1_dl_body *dlb)
> > +{
> > +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> > +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> > +	struct vsp1_rwpf *rpf0 = pipe->inputs[0];
> > +	int ret;
> > +
> > +	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, rpf0, isp_fourcc, width,
> > +					    height);
> > +	if (ret)
> > +		return ret;
> > +
> > +	rpf0->format.plane_fmt[0].bytesperline = stride;
> > +
> > +	/*
> > +	 * Connect RPF0 to the IIF sink pad corresponding to the config or image
> > +	 * path.
> > +	 */
> > +	rpf0->entity.sink_pad = iif_sink_pad;
> > +
> > +	pipe->part_table[0].rpf[0].width = width;
> > +	pipe->part_table[0].rpf[0].height = height;
> > +
> > +	rpf0->mem.addr[0] = addr;
> > +	rpf0->mem.addr[1] = 0;
> > +	rpf0->mem.addr[2] = 0;
> > +
> > +	vsp1_entity_route_setup(&rpf0->entity, pipe, dlb);
> > +	vsp1_entity_configure_stream(&rpf0->entity, rpf0->entity.state, pipe,
> > +				     dl, dlb);
> > +	vsp1_entity_configure_partition(&rpf0->entity, pipe,
> > +					&pipe->part_table[0], dl, dlb);
> > +
> > +	return 0;
> > +}
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Interrupt handling
> > + */
> > +static void vsp1_vspx_pipeline_frame_end(struct vsp1_pipeline *pipe,
> > +					 unsigned int completion)
> > +{
> > +	struct vsp1_vspx_pipeline *vspx_pipe = to_vsp1_vspx_pipeline(pipe);
> > +	struct vsp1_vspx *vspx = to_vsp1_vspx(vspx_pipe);
> > +
> > +	if (vspx_pipe->vspx_frame_end)
> > +		vspx_pipe->vspx_frame_end(vspx_pipe->frame_end_data);
> > +
> > +	/*
> > +	 * Set the pipeline state to stopped to ensure the next call to
> > +	 * vsp1_pipeline_run() actually starts the VSPX.
> > +	 */
> > +	scoped_guard(spinlock_irqsave, &pipe->irqlock) {
> > +		pipe->state = VSP1_PIPELINE_STOPPED;
> > +	}
> > +
> > +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> > +		vspx_pipe->processing = false;
> > +	}
> > +
> > +	/* Try schedule a new job from the queue. */
> > +	vsp1_isp_job_run(vspx->vsp1->dev);
> > +}
> > +
> > +/* -----------------------------------------------------------------------------
> > + * ISP Driver API (include/media/vsp1.h)
> > + */
> > +
> > +/**
> > + * vsp1_isp_init() - Initialize the VSPX
> > + *
> > + * @dev: The VSP1 struct device
> > + *
> > + * Return: %0 on success or a negative error code on failure
> > + */
> > +int vsp1_isp_init(struct device *dev)
> > +{
> > +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> > +
> > +	if (!vsp1)
> > +		return -EPROBE_DEFER;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(vsp1_isp_init);
> > +
> > +/**
> > + * vsp1_isp_get_bus_master - Get VSPX bus master
> > + *
> > + * The VSPX access memory through an FCPX instance. When allocating memory
> > + * buffers that will have to be accessed by the VSPX the 'struct device' of
> > + * the FCPX should be used. Use this function to get a reference to it.
> > + *
> > + * @dev: The VSP1 struct device
> > + *
> > + * Return: a pointer to the bus master's device
> > + */
> > +struct device *vsp1_isp_get_bus_master(struct device *dev)
> > +{
> > +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> > +
> > +	if (!vsp1)
> > +		return ERR_PTR(-ENODEV);
> > +
> > +	return vsp1->bus_master;
> > +}
> > +EXPORT_SYMBOL_GPL(vsp1_isp_get_bus_master);
> > +
> > +/**
> > + * vsp1_isp_alloc_buffers - Allocate buffers in the VSPX address space
> > + *
> > + * Allocate buffers that will be later accessed by the VSPX.
> > + *
> > + * @dev: The VSP1 struct device
> > + * @size: The size of the buffer to be allocated by the VSPX
> > + * @buffer_desc: The allocated buffer description, will be filled with the
> > + *		 buffer CPU-mapped address and the bus address
> > + *
> > + * Return: %0 on success or a negative error code on failure
> > + */
> > +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> > +			   struct vsp1_isp_buffer_desc *buffer_desc)
> > +{
> > +	struct device *bus_master = vsp1_isp_get_bus_master(dev);
> > +
> > +	if (IS_ERR_OR_NULL(bus_master))
> > +		return -ENODEV;
> > +
> > +	buffer_desc->cpu_addr = dma_alloc_coherent(bus_master, size,
> > +						   &buffer_desc->dma_addr,
> > +						   GFP_KERNEL);
> > +	if (IS_ERR_OR_NULL(buffer_desc->cpu_addr))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(vsp1_isp_alloc_buffers);
> > +
> > +/**
> > + * vsp1_isp_configure - Configure the VSPX with the RAW image format
> > + *
> > + * Apply to the VSPX RPF/WPF the size of the RAW image that will be transferred
> > + * to the ISP.
> > + *
> > + * @dev: The VSP1 struct device
> > + * @fmt: The RAW image format description
> > + *
> > + * Return: %0 on success or a negative error code on failure
> > + */
> > +int vsp1_isp_configure(struct device *dev,
> > +		       const struct v4l2_pix_format_mplane *fmt)
> > +{
> > +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> > +	struct vsp1_vspx_pipeline *vspx_pipe;
> > +	struct vsp1_pipeline *pipe;
> > +	int ret;
> > +
> > +	vspx_pipe = &vsp1->vspx->pipe;
> > +	pipe = &vspx_pipe->pipe;
> > +
> > +	/*
> > +	 * Apply the same format to the RPF0 and WPF0 so that the partition
> > +	 * calculation results in a single partition.
> > +	 */
> > +	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->inputs[0],
> > +					    fmt->pixelformat, fmt->width,
> > +					    fmt->height);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->output, fmt->pixelformat,
> > +					    fmt->width, fmt->height);
> > +	if (ret)
> > +		return ret;
> > +
> > +	vsp1_pipeline_calculate_partition(pipe, &pipe->part_table[0],
> > +					  fmt->width, 0);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(vsp1_isp_configure);
> > +
> > +static void vsp1_vspx_release_jobs(struct vsp1_device *vsp1)
> > +{
> > +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> > +	struct vsp1_vspx_job *job, *tmp;
> > +
> > +	guard(spinlock_irqsave)(&vspx_pipe->jobs_lock);
> > +
> > +	list_for_each_entry_safe(job, tmp, &vspx_pipe->jobs, job_queue) {
> > +		list_del(&job->job_queue);
> > +		vsp1_dl_list_put(job->dl);
> > +		kfree(job);
> > +	}
> > +}
> > +
> > +/**
> > + * vsp1_isp_start_streaming - Start processing VSPX jobs
> > + *
> > + * Start the VSPX and prepare for accepting buffer transfer job requests.
> > + *
> > + * @dev: The VSP1 struct device
> > + * @frame_end: The frame end callback description
> > + * @enable: The enable/disable streaming flag
> > + *
> > + * Return: %0 on success or a negative error code on failure
> > + */
> > +int vsp1_isp_start_streaming(struct device *dev,
> > +			     struct vsp1_vspx_frame_end *frame_end,
> > +			     bool enable)
> > +{
> > +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> > +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> > +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> > +	u32 value;
> > +	int ret;
> > +
> > +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> > +		if (vspx_pipe->enabled == enable)
> > +			return 0;
> > +
> > +		vspx_pipe->enabled = enable;
> > +	}
> > +
> > +	if (!enable) {
> > +		pipe->state = VSP1_PIPELINE_STOPPED;
> > +		vsp1_pipeline_stop(pipe);
> > +		vsp1_vspx_release_jobs(vsp1);
> > +		vspx_pipe->processing = false;
> > +		vspx_pipe->vspx_frame_end = NULL;
> > +		vsp1_dlm_reset(pipe->output->dlm);
> > +		vsp1_device_put(vsp1);
> > +		return 0;
> > +	}
> > +
> > +	if (!frame_end) {
> > +		ret = -EINVAL;
> > +		goto error_stop_pipe;
> > +	}
> > +
> > +	vspx_pipe->vspx_frame_end = frame_end->vspx_frame_end;
> > +	vspx_pipe->frame_end_data = frame_end->frame_end_data;
> > +
> > +	/* Make sure VSPX is not active. */
> > +	value = vsp1_read(vsp1, VI6_CMD(0));
> > +	if (value & VI6_CMD_STRCMD) {
> > +		dev_err(vsp1->dev,
> > +			"%s: Starting of WPF0 already reserved\n", __func__);
> > +		ret = -EBUSY;
> > +		goto error_stop_pipe;
> > +	}
> > +
> > +	value = vsp1_read(vsp1, VI6_STATUS);
> > +	if (value & VI6_STATUS_SYS_ACT(0)) {
> > +		dev_err(vsp1->dev,
> > +			"%s: WPF0 has not entered idle state\n", __func__);
> > +		ret = -EBUSY;
> > +		goto error_stop_pipe;
> > +	}
> > +
> > +	/* Enable the VSP1 and prepare for streaming. */
> > +	vsp1_pipeline_dump(pipe, "VSPX job");
> > +
> > +	ret = vsp1_device_get(vsp1);
> > +	if (ret < 0)
> > +		goto error_stop_pipe;
> > +
> > +	return 0;
> > +
> > +error_stop_pipe:
> > +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> > +		vspx_pipe->enabled = false;
> > +	}
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(vsp1_isp_start_streaming);
> > +
> > +/**
> > + * vsp1_vspx_job_prepare - Prepare a display list with the content of the buffer
> > + *
> > + * @dev: The VSP1 struct device
> > + * @job: The job description
> > + *
> > + * Return: %0 on success or a negative error code on failure
> > + */
> > +int vsp1_isp_job_prepare(struct device *dev,
> > +			 const struct vsp1_isp_job_desc *desc)
> > +{
> > +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> > +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> > +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> > +	const struct v4l2_pix_format_mplane *pix_mp;
> > +	struct vsp1_dl_list *second_dl = NULL;
> > +	struct vsp1_vspx_job *job;
> > +	struct vsp1_dl_body *dlb;
> > +	struct vsp1_dl_list *dl;
> > +	int ret;
> > +
> > +	/*
> > +	 * Populate a display list and append it to the jobs queue.
> > +	 * Memory is released when the job is consumed.
> > +	 */
> > +	job = kmalloc(sizeof(*job), GFP_KERNEL);
> > +	if (!job)
> > +		return -ENOMEM;
> > +
> > +	/*
> > +	 * Transfer the buffers described in the job: (optional) ConfigDMA and
> > +	 * RAW image.
> > +	 */
> > +
> > +	job->dl = vsp1_dl_list_get(pipe->output->dlm);
> > +	if (!job->dl) {
> > +		ret = -ENOMEM;
> > +		goto error_free_job;
> > +	}
> > +
> > +	dl = job->dl;
> > +	dlb = vsp1_dl_list_get_body0(dl);
> > +
> > +	/* Disable RPF1. */
> > +	vsp1_dl_body_write(dlb, vsp1->rpf[1]->entity.route->reg,
> > +			   VI6_DPR_NODE_UNUSED);
> > +
> > +	/* Configure IIF routing and enable IIF function */
> > +	vsp1_entity_route_setup(pipe->iif, pipe, dlb);
> > +	vsp1_entity_configure_stream(pipe->iif, pipe->iif->state, pipe,
> > +				     dl, dlb);
> > +
> > +	/* Configure WPF0 to enable RPF0 as source*/
> > +	vsp1_entity_route_setup(&pipe->output->entity, pipe, dlb);
> > +	vsp1_entity_configure_stream(&pipe->output->entity,
> > +				     pipe->output->entity.state, pipe,
> > +				     dl, dlb);
> > +
> > +	if (desc->config.pairs) {
> > +		/*
> > +		 * Configure RPF0 for config data. Transfer the number of
> > +		 * configuration pairs plus 2 words for the header.
> > +		 */
> > +		ret = vsp1_vspx_rpf0_configure(vsp1, desc->config.mem,
> > +					       V4L2_PIX_FMT_XBGR32,
> > +					       desc->config.pairs * 2 + 2, 1,
> > +					       desc->config.pairs * 2 + 2,
> > +					       VSPX_IIF_SINK_PAD_CONFIG,
> > +					       dl, dlb);
> > +		if (ret)
> > +			goto error_put_dl;
> > +
> > +		second_dl = vsp1_dl_list_get(pipe->output->dlm);
> > +		if (!second_dl) {
> > +			ret = -ENOMEM;
> > +			goto error_put_dl;
> > +		}
> > +
> > +		dl = second_dl;
> > +		dlb = vsp1_dl_list_get_body0(dl);
> > +	}
> > +
> > +	/* Configure RPF0 for RAW image transfer. */
> > +	pix_mp = &desc->img.fmt.fmt.pix_mp;
> > +	ret = vsp1_vspx_rpf0_configure(vsp1, desc->img.mem,
> > +				       pix_mp->pixelformat,
> > +				       pix_mp->width, pix_mp->height,
> > +				       pix_mp->plane_fmt[0].bytesperline,
> > +				       VSPX_IIF_SINK_PAD_IMG, dl, dlb);
> > +	if (ret)
> > +		goto error_put_dl;
> > +
> > +	if (second_dl)
> > +		vsp1_dl_list_add_chain(job->dl, second_dl);
> > +
> > +	scoped_guard(spinlock_irqsave, &vspx_pipe->jobs_lock) {
> > +		list_add_tail(&job->job_queue, &vspx_pipe->jobs);
> > +	}
> > +
> > +	return 0;
> > +
> > +error_put_dl:
> > +	if (second_dl)
> > +		vsp1_dl_list_put(second_dl);
> > +	vsp1_dl_list_put(job->dl);
> > +error_free_job:
> > +	kfree(job);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(vsp1_isp_job_prepare);
> > +
> > +/**
> > + * vsp1_isp_job_run - Run a buffer transfer on behalf of the ISP
> > + *
> > + * @dev: The VSP1 struct device
> > + */
> > +void vsp1_isp_job_run(struct device *dev)
> > +{
> > +	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> > +	struct vsp1_vspx_pipeline *vspx_pipe = &vsp1->vspx->pipe;
> > +	struct vsp1_pipeline *pipe = &vspx_pipe->pipe;
> > +	struct vsp1_vspx_job *job;
> > +
> > +	scoped_guard(spinlock_irqsave, &vspx_pipe->vspx_lock) {
> > +
> > +		if (vspx_pipe->processing)
> > +			return;
> > +
> > +		/* Extract one job, if available, from the jobs list. */
> > +		scoped_guard(spinlock_irqsave, &vspx_pipe->jobs_lock) {
> > +			job = list_first_entry_or_null(&vspx_pipe->jobs,
> > +						       struct vsp1_vspx_job,
> > +						       job_queue);
> > +			if (!job)
> > +				return;
> > +
> > +			list_del(&job->job_queue);
> > +		}
> > +
> > +		vspx_pipe->processing = true;
> > +		vsp1_dl_list_commit(job->dl, 0);
> > +		kfree(job);
> > +	}
> > +
> > +	/* Trigger VSPX processing by setting VI6_CMD[STRCMD]. */
> > +	scoped_guard(spinlock_irqsave, &pipe->irqlock) {
> > +		vsp1_pipeline_run(pipe);
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(vsp1_isp_job_run);
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Initialization and cleanup
> > + */
> > +
> > +int vsp1_vspx_init(struct vsp1_device *vsp1)
> > +{
> > +	struct vsp1_vspx_pipeline *vspx_pipe;
> > +	struct vsp1_pipeline *pipe;
> > +
> > +	vsp1->vspx = devm_kzalloc(vsp1->dev, sizeof(*vsp1->vspx), GFP_KERNEL);
> > +	if (!vsp1->vspx)
> > +		return -ENOMEM;
> > +
> > +	vsp1->vspx->vsp1 = vsp1;
> > +
> > +	vspx_pipe = &vsp1->vspx->pipe;
> > +	vspx_pipe->processing = false;
> > +	vspx_pipe->enabled = false;
> > +
> > +	pipe = &vspx_pipe->pipe;
> > +
> > +	vsp1_pipeline_init(pipe);
> > +
> > +	pipe->partitions = 1;
> > +	pipe->part_table = &vspx_pipe->partition;
> > +	pipe->interlaced = false;
> > +	pipe->frame_end = vsp1_vspx_pipeline_frame_end;
> > +
> > +	INIT_LIST_HEAD(&vspx_pipe->jobs);
> > +	spin_lock_init(&vspx_pipe->vspx_lock);
> > +	spin_lock_init(&vspx_pipe->jobs_lock);
> > +
> > +	/*
> > +	 * Initialize RPF0 as inputs for VSPX and use it unconditionally for
> > +	 * now.
> > +	 */
> > +	pipe->inputs[0] = vsp1->rpf[0];
> > +	pipe->inputs[0]->entity.pipe = pipe;
> > +	pipe->inputs[0]->entity.sink = &vsp1->iif->entity;
> > +	vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->inputs[0],
> > +				      vspx_default_fmt.pixelformat,
> > +				      vspx_default_fmt.width,
> > +				      vspx_default_fmt.height);
> > +	list_add(&pipe->inputs[0]->entity.list_pipe, &pipe->entities);
> > +
> > +	pipe->iif = &vsp1->iif->entity;
> > +	pipe->iif->pipe = pipe;
> > +	pipe->iif->sink = &vsp1->wpf[0]->entity;
> > +	list_add(&pipe->iif->list_pipe, &pipe->entities);
> > +
> > +	pipe->output = vsp1->wpf[0];
> > +	pipe->output->entity.pipe = pipe;
> > +	vsp1_vspx_rwpf_set_subdev_fmt(vsp1, pipe->output,
> > +				      vspx_default_fmt.pixelformat,
> > +				      vspx_default_fmt.width,
> > +				      vspx_default_fmt.height);
> > +	list_add(&pipe->output->entity.list_pipe, &pipe->entities);
> > +
> > +	return 0;
> > +}
> > +
> > +void vsp1_vspx_cleanup(struct vsp1_device *vsp1)
> > +{
> > +	struct vsp1_vspx_pipeline *vspx_pipe;
> > +
> > +	vspx_pipe = &vsp1->vspx->pipe;
> > +
> > +	vsp1_vspx_release_jobs(vsp1);
> > +}
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_vspx.h b/drivers/media/platform/renesas/vsp1/vsp1_vspx.h
> > new file mode 100644
> > index 000000000000..2f68f043075e
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_vspx.h
> > @@ -0,0 +1,86 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * vsp1_vspx.h  --  R-Car Gen 4 VSPX
> > + *
> > + * Copyright (C) 2025 Ideas On Board Oy
> > + * Copyright (C) 2025 Renesas Electronics Corporation
> > + */
> > +#ifndef __VSP1_VSPX_H__
> > +#define __VSP1_VSPX_H__
> > +
> > +#include <linux/container_of.h>
> > +#include <linux/list.h>
> > +#include <linux/spinlock.h>
> > +
> > +#include <media/vsp1.h>
> > +
> > +#include "vsp1.h"
> > +#include "vsp1_pipe.h"
> > +
> > +/* Pixel format for ConfigDMA buffers. */
> > +#define V4L2_META_FMT_RCAR_V4H	v4l2_fourcc('R', 'C', 'A', '4')
> > +
> > +/*
> > + * struct vsp1_vspx_pipeline - VSPX pipeline
> > + * @pipe: the VSP1 pipeline
> > + * @partition: the pre-calculated partition used by the pipeline
> > + * @vspx_lock: protect access to the VSPX configuration
> > + * @processing: VSPX busy flag
> > + * @jobs_lock: protect the jobs queue
> > + * @jobs: jobs queue
> > + * @vspx_frame_end: frame end callback
> > + * @frame_end_data: data for the frame end callback
> > + */
> > +struct vsp1_vspx_pipeline {
> > +	struct vsp1_pipeline pipe;
> > +	struct vsp1_partition partition;
> > +
> > +	/* Protects the pipeline configuration */
> > +	spinlock_t vspx_lock;
> > +	bool processing;
> > +	bool enabled;
> > +
> > +	/* Protects the jobs list */
> > +	spinlock_t jobs_lock;
> > +	struct list_head jobs;
> > +
> > +	void (*vspx_frame_end)(void *frame_end_data);
> > +	void *frame_end_data;
> > +};
> > +
> > +static inline struct vsp1_vspx_pipeline *
> > +to_vsp1_vspx_pipeline(struct vsp1_pipeline *pipe)
> > +{
> > +	return container_of(pipe, struct vsp1_vspx_pipeline, pipe);
> > +}
> > +
> > +/*
> > + * struct vsp1_vspx_job - VSPX transfer job
> > + * @dl: Display list populated by vsp1_isp_job_prepare
> > + * @job_queue: List handle
> > + */
> > +struct vsp1_vspx_job {
> > +	struct vsp1_dl_list *dl;
> > +	struct list_head job_queue;
> > +};
> > +
> > +/*
> > + * struct vsp1_vspx - VSPX device
> > + * @vsp1: the VSP1 device
> > + * @pipe: the VSPX pipeline
> > + */
> > +struct vsp1_vspx {
> > +	struct vsp1_device *vsp1;
> > +	struct vsp1_vspx_pipeline pipe;
> > +};
> > +
> > +static inline struct vsp1_vspx *
> > +to_vsp1_vspx(struct vsp1_vspx_pipeline *vspx_pipe)
> > +{
> > +	return container_of(vspx_pipe, struct vsp1_vspx, pipe);
> > +}
> > +
> > +int vsp1_vspx_init(struct vsp1_device *vsp1);
> > +void vsp1_vspx_cleanup(struct vsp1_device *vsp1);
> > +
> > +#endif /* __VSP1_VSPX_H__ */
> > diff --git a/include/media/vsp1.h b/include/media/vsp1.h
> > index 48f4a5023d81..68e4d9891dff 100644
> > --- a/include/media/vsp1.h
> > +++ b/include/media/vsp1.h
> > @@ -9,12 +9,17 @@
> >  #ifndef __MEDIA_VSP1_H__
> >  #define __MEDIA_VSP1_H__
> >  
> > +#include <linux/list.h>
> >  #include <linux/scatterlist.h>
> >  #include <linux/types.h>
> >  #include <linux/videodev2.h>
> >  
> >  struct device;
> >  
> > +/*******************************************************************************
> > + * VSP1 DU interface
> > + */
> > +
> >  int vsp1_du_init(struct device *dev);
> >  
> >  #define VSP1_DU_STATUS_COMPLETE		BIT(0)
> > @@ -117,4 +122,72 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
> >  int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt);
> >  void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt);
> >  
> > +/*******************************************************************************
> > + * VSP1 ISP interface
> > + */
> > +
> > +/**
> > + * struct vsp1_isp_buffer_desc - Describe a buffer allocated by VSPX
> > + *
> > + * @cpu_addr: CPU-mapped address of a buffer allocated by VSPX
> > + * @dma_addr: bus address of a buffer allocated by VSPX
> > + */
> > +struct vsp1_isp_buffer_desc {
> > +	void *cpu_addr;
> > +	dma_addr_t dma_addr;
> > +};
> > +
> > +/**
> > + * struct vsp1_isp_job_desc - Describe a VSPX buffer transfer request
> > + *
> > + * Describe a transfer request for the VSPX to perform on behalf of the ISP.
> > + * The transfer job descriptor contains an optional ConfigDMA buffer and
> > + * one RAW image buffer. Set config.pairs to 0 if no ConfigDMA buffer should
> > + * be transferred.
> > + *
> > + * @config: ConfigDMA buffer
> > + * @config.pairs: number of reg-value pairs in the ConfigDMA buffer
> > + * @config.mem: bus address of the ConfigDMA buffer
> > + * @img: RAW image buffer
> > + * @img.fmt: RAW image format
> > + * @img.mem: bus address of the RAW image buffer
> > + */
> > +struct vsp1_isp_job_desc {
> > +	struct {
> > +		unsigned int pairs;
> > +		dma_addr_t mem;
> > +	} config;
> > +	struct {
> > +		struct v4l2_format fmt;
> 
> I wonder if we can't drop this member and use the format passed into 
> vsp1_isp_configure()? I think if these two formats ever differed bad 
> things would happen no?

I think we should do it the other way around, and drop the configure
function. Each job could use a different format.

> > +		dma_addr_t mem;
> > +	} img;
> > +};
> > +
> > +/**
> > + * struct vsp1_vspx_frame_end - VSPX frame end callback data
> > + *
> > + * @vspx_frame_end: Frame end callback. Called after a transfer job has been
> > + *		    completed. If the job includes both a ConfigDMA and a
> > + *		    RAW image, the callback is called after both have been
> > + *		    transferred
> > + * @frame_end_data: Frame end callback data, passed to vspx_frame_end
> > + */
> > +struct vsp1_vspx_frame_end {
> > +	void (*vspx_frame_end)(void *data);
> > +	void *frame_end_data;
> > +};
> > +
> > +int vsp1_isp_init(struct device *dev);
> > +struct device *vsp1_isp_get_bus_master(struct device *dev);
> > +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> > +			   struct vsp1_isp_buffer_desc *buffer_desc);
> > +int vsp1_isp_configure(struct device *dev,
> > +		       const struct v4l2_pix_format_mplane *fmt);
> > +int vsp1_isp_start_streaming(struct device *dev,
> > +			     struct vsp1_vspx_frame_end *frame_end,
> > +			     bool enable);
> > +int vsp1_isp_job_prepare(struct device *dev,
> > +			 const struct vsp1_isp_job_desc *desc);
> > +void vsp1_isp_job_run(struct device *dev);
> > +
> >  #endif /* __MEDIA_VSP1_H__ */
> > 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v7 0/5] media: renesas: vsp1: Add support for VSPX and IIF

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patches.

On Tue, Apr 01, 2025 at 04:22:00PM +0200, Jacopo Mondi wrote:
> The VSPX is a VSP2 function that reads data from
> external memory using two RPF instances and feed it to the ISP.
> 
> The VSPX includes an IIF unit (ISP InterFace) modeled in the vsp1 driver
> as a new, simple, entity type.
> 
> IIF is part of VSPX, a version of the VSP2 IP specialized for ISP
> interfacing. To prepare to support VSPX, support IIF first by
> introducing a new entity and by adjusting the RPF/WPF drivers to
> operate correctly when an IIF is present.

I think patches 1/5 to 4/5 are ready to be merged, but 5/5 needs more
work. Please let me know if I can take the first four patches in my tree
already.

> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---
> Changes in v7:
> - Include VSPX driver in the series
> - Use existing VSP1 formats and remove patches extending formats on RPF
> - Rework VSPX driver to split jobs creation and scheduling in two
>   different API entry points
> - Fix VSPX stride using the user provided bytesperline and using the
>   buffer size for ConfigDMA buffers
> - Link to v6: https://lore.kernel.org/r/20250321-v4h-iif-v6-0-361e9043026a@ideasonboard.com
> 
> Changes in v6:
> - Little cosmetic change as suggested by Laurent
> - Collect tags
> - Link to v5: https://lore.kernel.org/r/20250319-v4h-iif-v5-0-0a10456d792c@ideasonboard.com
> 
> Changes in v5:
> - Drop additional empty line 5/6
> - Link to v4: https://lore.kernel.org/r/20250318-v4h-iif-v4-0-10ed4c41c195@ideasonboard.com
> 
> Changes in v4:
> - Fix SWAP bits for RAW10, RAW12 and RAW16
> - Link to v3: https://lore.kernel.org/r/20250317-v4h-iif-v3-0-63aab8982b50@ideasonboard.com
> 
> Changes in v3:
> - Drop 2/6 from v2
> - Add 5/7 to prepare for a new implementation of 6/7
> - Individual changelog per patch
> - Add 7/7
> - Link to v2: https://lore.kernel.org/r/20250224-v4h-iif-v2-0-0305e3c1fe2d@ideasonboard.com
> 
> Changes in v2:
> - Collect tags
> - Address review comments from Laurent, a lot of tiny changes here and
>   there but no major redesign worth an entry in the patchset changelog
> 
> ---
> Jacopo Mondi (5):
>       media: vsp1: Add support IIF ISP Interface
>       media: vsp1: dl: Use singleshot DL for VSPX
>       media: vsp1: wpf: Propagate vsp1_rwpf_init_ctrls()
>       media: vsp1: rwpf: Support operations with IIF
>       media: vsp1: Add VSPX support
> 
>  drivers/media/platform/renesas/vsp1/Makefile      |   3 +-
>  drivers/media/platform/renesas/vsp1/vsp1.h        |   4 +
>  drivers/media/platform/renesas/vsp1/vsp1_dl.c     |   7 +-
>  drivers/media/platform/renesas/vsp1/vsp1_drv.c    |  24 +-
>  drivers/media/platform/renesas/vsp1/vsp1_entity.c |   8 +
>  drivers/media/platform/renesas/vsp1/vsp1_entity.h |   1 +
>  drivers/media/platform/renesas/vsp1/vsp1_iif.c    | 121 +++++
>  drivers/media/platform/renesas/vsp1/vsp1_iif.h    |  29 ++
>  drivers/media/platform/renesas/vsp1/vsp1_pipe.c   |   1 +
>  drivers/media/platform/renesas/vsp1/vsp1_pipe.h   |   1 +
>  drivers/media/platform/renesas/vsp1/vsp1_regs.h   |   9 +
>  drivers/media/platform/renesas/vsp1/vsp1_rpf.c    |   9 +-
>  drivers/media/platform/renesas/vsp1/vsp1_vspx.c   | 604 ++++++++++++++++++++++
>  drivers/media/platform/renesas/vsp1/vsp1_vspx.h   |  86 +++
>  drivers/media/platform/renesas/vsp1/vsp1_wpf.c    |  24 +-
>  include/media/vsp1.h                              |  73 +++
>  16 files changed, 991 insertions(+), 13 deletions(-)
> ---
> base-commit: f2151613e040973c868d28c8b00885dfab69eb75
> change-id: 20250123-v4h-iif-a1dda640c95d

-- 
Regards,

Laurent Pinchart

Re: [PATCH v7 0/5] media: renesas: vsp1: Add support for VSPX and IIF

[Jacopo Mondi]

Hi Laurent

On Thu, Apr 24, 2025 at 12:14:09AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patches.
>
> On Tue, Apr 01, 2025 at 04:22:00PM +0200, Jacopo Mondi wrote:
> > The VSPX is a VSP2 function that reads data from
> > external memory using two RPF instances and feed it to the ISP.
> >
> > The VSPX includes an IIF unit (ISP InterFace) modeled in the vsp1 driver
> > as a new, simple, entity type.
> >
> > IIF is part of VSPX, a version of the VSP2 IP specialized for ISP
> > interfacing. To prepare to support VSPX, support IIF first by
> > introducing a new entity and by adjusting the RPF/WPF drivers to
> > operate correctly when an IIF is present.
>
> I think patches 1/5 to 4/5 are ready to be merged, but 5/5 needs more
> work. Please let me know if I can take the first four patches in my tree
> already.

I think so, yes.

If we later find out we need to modify anything, we could do that on
top, but at the moment I think all changes will be in the VSPX driver
itself, which I will re-send separately.

Thanks
  j

>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> > ---
> > Changes in v7:
> > - Include VSPX driver in the series
> > - Use existing VSP1 formats and remove patches extending formats on RPF
> > - Rework VSPX driver to split jobs creation and scheduling in two
> >   different API entry points
> > - Fix VSPX stride using the user provided bytesperline and using the
> >   buffer size for ConfigDMA buffers
> > - Link to v6: https://lore.kernel.org/r/20250321-v4h-iif-v6-0-361e9043026a@ideasonboard.com
> >
> > Changes in v6:
> > - Little cosmetic change as suggested by Laurent
> > - Collect tags
> > - Link to v5: https://lore.kernel.org/r/20250319-v4h-iif-v5-0-0a10456d792c@ideasonboard.com
> >
> > Changes in v5:
> > - Drop additional empty line 5/6
> > - Link to v4: https://lore.kernel.org/r/20250318-v4h-iif-v4-0-10ed4c41c195@ideasonboard.com
> >
> > Changes in v4:
> > - Fix SWAP bits for RAW10, RAW12 and RAW16
> > - Link to v3: https://lore.kernel.org/r/20250317-v4h-iif-v3-0-63aab8982b50@ideasonboard.com
> >
> > Changes in v3:
> > - Drop 2/6 from v2
> > - Add 5/7 to prepare for a new implementation of 6/7
> > - Individual changelog per patch
> > - Add 7/7
> > - Link to v2: https://lore.kernel.org/r/20250224-v4h-iif-v2-0-0305e3c1fe2d@ideasonboard.com
> >
> > Changes in v2:
> > - Collect tags
> > - Address review comments from Laurent, a lot of tiny changes here and
> >   there but no major redesign worth an entry in the patchset changelog
> >
> > ---
> > Jacopo Mondi (5):
> >       media: vsp1: Add support IIF ISP Interface
> >       media: vsp1: dl: Use singleshot DL for VSPX
> >       media: vsp1: wpf: Propagate vsp1_rwpf_init_ctrls()
> >       media: vsp1: rwpf: Support operations with IIF
> >       media: vsp1: Add VSPX support
> >
> >  drivers/media/platform/renesas/vsp1/Makefile      |   3 +-
> >  drivers/media/platform/renesas/vsp1/vsp1.h        |   4 +
> >  drivers/media/platform/renesas/vsp1/vsp1_dl.c     |   7 +-
> >  drivers/media/platform/renesas/vsp1/vsp1_drv.c    |  24 +-
> >  drivers/media/platform/renesas/vsp1/vsp1_entity.c |   8 +
> >  drivers/media/platform/renesas/vsp1/vsp1_entity.h |   1 +
> >  drivers/media/platform/renesas/vsp1/vsp1_iif.c    | 121 +++++
> >  drivers/media/platform/renesas/vsp1/vsp1_iif.h    |  29 ++
> >  drivers/media/platform/renesas/vsp1/vsp1_pipe.c   |   1 +
> >  drivers/media/platform/renesas/vsp1/vsp1_pipe.h   |   1 +
> >  drivers/media/platform/renesas/vsp1/vsp1_regs.h   |   9 +
> >  drivers/media/platform/renesas/vsp1/vsp1_rpf.c    |   9 +-
> >  drivers/media/platform/renesas/vsp1/vsp1_vspx.c   | 604 ++++++++++++++++++++++
> >  drivers/media/platform/renesas/vsp1/vsp1_vspx.h   |  86 +++
> >  drivers/media/platform/renesas/vsp1/vsp1_wpf.c    |  24 +-
> >  include/media/vsp1.h                              |  73 +++
> >  16 files changed, 991 insertions(+), 13 deletions(-)
> > ---
> > base-commit: f2151613e040973c868d28c8b00885dfab69eb75
> > change-id: 20250123-v4h-iif-a1dda640c95d
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v7 5/5] media: vsp1: Add VSPX support

[Jacopo Mondi]

Hi Laurent

On Thu, Apr 24, 2025 at 12:10:35AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Apr 01, 2025 at 04:22:05PM +0200, Jacopo Mondi wrote:
> > Add support for VSPX, a specialized version of the VSP2 that
> > transfer data to the ISP. The VSPX is composed by two RPF units
>
> It seems you forgot to take comments from v2 into account.
>

Are you referring to the commit message alone ? Or is there anything
else ?

> > to read data from external memory and an IIF instance that performs
> > transfer towards the ISP.
> >

[snip]

>
> > + *		 buffer CPU-mapped address and the bus address
> > + *
> > + * Return: %0 on success or a negative error code on failure
> > + */
> > +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> > +			   struct vsp1_isp_buffer_desc *buffer_desc)
> > +{
> > +	struct device *bus_master = vsp1_isp_get_bus_master(dev);
> > +
> > +	if (IS_ERR_OR_NULL(bus_master))
> > +		return -ENODEV;
> > +
> > +	buffer_desc->cpu_addr = dma_alloc_coherent(bus_master, size,
> > +						   &buffer_desc->dma_addr,
> > +						   GFP_KERNEL);
> > +	if (IS_ERR_OR_NULL(buffer_desc->cpu_addr))
> > +		return -EINVAL;
>
> As commented by Alok, this should be
>
> 	if (!buffer_desc->cpu_addr)
> 		return -ENOMEM;

Done, thanks Alok for the comment

>
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(vsp1_isp_alloc_buffers);
>
> Where is the buffer freed ?
>

Right, I presume a call to dma_free_coherent() is needed in the
vb2 buf_cleanup() operation call path ?

Re: [PATCH v7 5/5] media: vsp1: Add VSPX support

[Laurent Pinchart]

On Thu, Apr 24, 2025 at 03:18:23PM +0200, Jacopo Mondi wrote:
> On Thu, Apr 24, 2025 at 12:10:35AM +0300, Laurent Pinchart wrote:
> > On Tue, Apr 01, 2025 at 04:22:05PM +0200, Jacopo Mondi wrote:
> > > Add support for VSPX, a specialized version of the VSP2 that
> > > transfer data to the ISP. The VSPX is composed by two RPF units
> >
> > It seems you forgot to take comments from v2 into account.
> >
> 
> Are you referring to the commit message alone ? Or is there anything
> else ?

Yes, just the commit message.

> > > to read data from external memory and an IIF instance that performs
> > > transfer towards the ISP.
> > >
> 
> [snip]
> 
> > > + *		 buffer CPU-mapped address and the bus address
> > > + *
> > > + * Return: %0 on success or a negative error code on failure
> > > + */
> > > +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> > > +			   struct vsp1_isp_buffer_desc *buffer_desc)
> > > +{
> > > +	struct device *bus_master = vsp1_isp_get_bus_master(dev);
> > > +
> > > +	if (IS_ERR_OR_NULL(bus_master))
> > > +		return -ENODEV;
> > > +
> > > +	buffer_desc->cpu_addr = dma_alloc_coherent(bus_master, size,
> > > +						   &buffer_desc->dma_addr,
> > > +						   GFP_KERNEL);
> > > +	if (IS_ERR_OR_NULL(buffer_desc->cpu_addr))
> > > +		return -EINVAL;
> >
> > As commented by Alok, this should be
> >
> > 	if (!buffer_desc->cpu_addr)
> > 		return -ENOMEM;
> 
> Done, thanks Alok for the comment
> 
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(vsp1_isp_alloc_buffers);
> >
> > Where is the buffer freed ?
> >
> 
> Right, I presume a call to dma_free_coherent() is needed in the
> vb2 buf_cleanup() operation call path ?

It can be done by the ISP driver (directly, or through a function
exported by the VSP driver, to match vsp1_isp_alloc_buffers()), or be
done by the VSP driver. The important part is to document the design
properly.

-- 
Regards,

Laurent Pinchart