linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures
@ 2025-07-03 22:38 Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 1/8] media: renesas: vsp1: Store supported media bus codes in vsp1_entity Laurent Pinchart
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-07-03 22:38 UTC (permalink / raw)
  To: linux-media
  Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen

Hello,

This series fixes all v4l2-compliance failures in the VSP1 driver. The
v4l2-compliance log is long, and not that interesting as there's no
failure. Here's the executive summary, if anyone wants the full log,
please let me know.

----------------------------------------
root@buildroot ~ # v4l2-compliance -m /dev/media1
v4l2-compliance 1.31.0-5383, 64 bits, 64-bit time_t
v4l2-compliance SHA: 58fb1141ca7c 2025-07-03 19:50:58
        
Compliance test for vsp1 device /dev/media1:
        
Media Driver Info:
        Driver name      : vsp1
        Model            : VSP2-BD
        Serial           : 
        Bus info         : platform:fe960000.vsp
        Media version    : 6.16.0 
        Hardware revision: 0x01011504 (16848132)
        Driver version   : 6.16.0

[...]

        
Grand Total for vsp1 device /dev/media1: 751, Succeeded: 751, Failed: 0, Warnings: 0
----------------------------------------

The series has also been tested with the vsp-tests suite, which reported
no regression.

Compared to v1, I've addressed small review comments, and reworked patch
4/7 that became 4/8 and 5/8 in this version.

Laurent Pinchart (8):
  media: renesas: vsp1: Store supported media bus codes in vsp1_entity
  media: renesas: vsp1: Store size limits in vsp1_entity
  media: renesas: vsp1: Fix code checks in frame size enumeration
  media: renesas: vsp1: Fix crop left and top clamping on RPF
  media: renesas: vsp1: Fix crop width and height clamping on RPF
  media: renesas: vsp1: Fix RWPF media bus code and frame size
    enumeration
  media: renesas: vsp1: Fix format propagation on the BRX
  media: renesas: vsp1: Implement control events

 .../media/platform/renesas/vsp1/vsp1_brx.c    |  37 ++---
 .../media/platform/renesas/vsp1/vsp1_clu.c    |  47 ++-----
 .../media/platform/renesas/vsp1/vsp1_entity.c | 110 +++++++--------
 .../media/platform/renesas/vsp1/vsp1_entity.h |  21 +--
 .../media/platform/renesas/vsp1/vsp1_histo.c  |  27 ++--
 .../media/platform/renesas/vsp1/vsp1_histo.h  |   2 -
 .../media/platform/renesas/vsp1/vsp1_hsit.c   |  24 ++--
 .../media/platform/renesas/vsp1/vsp1_iif.c    |  39 ++----
 .../media/platform/renesas/vsp1/vsp1_lif.c    |  40 ++----
 .../media/platform/renesas/vsp1/vsp1_lut.c    |  47 ++-----
 .../media/platform/renesas/vsp1/vsp1_rpf.c    |   7 +-
 .../media/platform/renesas/vsp1/vsp1_rwpf.c   | 127 ++++++++++++++----
 .../media/platform/renesas/vsp1/vsp1_rwpf.h   |   6 +-
 .../media/platform/renesas/vsp1/vsp1_sru.c    |  63 ++++-----
 .../media/platform/renesas/vsp1/vsp1_uds.c    |  62 ++++-----
 .../media/platform/renesas/vsp1/vsp1_uif.c    |  40 ++----
 .../media/platform/renesas/vsp1/vsp1_wpf.c    |  13 +-
 17 files changed, 335 insertions(+), 377 deletions(-)


base-commit: c26e8dcd9d4e86d788c5bf7a5dd0ea70a95ab067
-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/8] media: renesas: vsp1: Store supported media bus codes in vsp1_entity
  2025-07-03 22:38 [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
@ 2025-07-03 22:38 ` Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 2/8] media: renesas: vsp1: Store size limits " Laurent Pinchart
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-07-03 22:38 UTC (permalink / raw)
  To: linux-media
  Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen

Most entities use the vsp1_subdev_enum_mbus_code() and
vsp1_subdev_set_pad_format() helper functions to implement the
corresponding subdev operations. Both helpers are given the list of
supported media bus codes as arguments, requiring each entity to
implement a wrapper.

Replace the function arguments with storing the supported media bus
codes in the vsp1_entity structure. This allows dropping most of the
.enum_mbus_code() wrappers from entities.

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

- Address IIF entity
---
 .../media/platform/renesas/vsp1/vsp1_brx.c    | 22 +++++++----------
 .../media/platform/renesas/vsp1/vsp1_clu.c    | 20 +++++-----------
 .../media/platform/renesas/vsp1/vsp1_entity.c | 21 +++++++---------
 .../media/platform/renesas/vsp1/vsp1_entity.h |  7 +++---
 .../media/platform/renesas/vsp1/vsp1_histo.c  | 18 ++++----------
 .../media/platform/renesas/vsp1/vsp1_histo.h  |  2 --
 .../media/platform/renesas/vsp1/vsp1_hsit.c   | 13 +++++++---
 .../media/platform/renesas/vsp1/vsp1_iif.c    | 12 +++-------
 .../media/platform/renesas/vsp1/vsp1_lif.c    | 20 +++++-----------
 .../media/platform/renesas/vsp1/vsp1_lut.c    | 20 +++++-----------
 .../media/platform/renesas/vsp1/vsp1_rwpf.c   | 24 ++++++++++---------
 .../media/platform/renesas/vsp1/vsp1_sru.c    | 20 ++++++----------
 .../media/platform/renesas/vsp1/vsp1_uds.c    | 20 ++++++----------
 .../media/platform/renesas/vsp1/vsp1_uif.c    | 20 +++++-----------
 14 files changed, 89 insertions(+), 150 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index 5fc2e5a3bb30..3890adc8073e 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -59,25 +59,17 @@ static const struct v4l2_ctrl_ops brx_ctrl_ops = {
  * V4L2 Subdevice Operations
  */
 
+static const unsigned int brx_codes[] = {
+	MEDIA_BUS_FMT_ARGB8888_1X32,
+	MEDIA_BUS_FMT_AYUV8_1X32,
+};
+
 /*
  * The BRx can't perform format conversion, all sink and source formats must be
  * identical. We pick the format on the first sink pad (pad 0) and propagate it
  * to all other pads.
  */
 
-static int brx_enum_mbus_code(struct v4l2_subdev *subdev,
-			      struct v4l2_subdev_state *sd_state,
-			      struct v4l2_subdev_mbus_code_enum *code)
-{
-	static const unsigned int codes[] = {
-		MEDIA_BUS_FMT_ARGB8888_1X32,
-		MEDIA_BUS_FMT_AYUV8_1X32,
-	};
-
-	return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, codes,
-					  ARRAY_SIZE(codes));
-}
-
 static int brx_enum_frame_size(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
 			       struct v4l2_subdev_frame_size_enum *fse)
@@ -262,7 +254,7 @@ static int brx_set_selection(struct v4l2_subdev *subdev,
 }
 
 static const struct v4l2_subdev_pad_ops brx_pad_ops = {
-	.enum_mbus_code = brx_enum_mbus_code,
+	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
 	.enum_frame_size = brx_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = brx_set_format,
@@ -416,6 +408,8 @@ struct vsp1_brx *vsp1_brx_create(struct vsp1_device *vsp1,
 	brx->base = type == VSP1_ENTITY_BRU ? VI6_BRU_BASE : VI6_BRS_BASE;
 	brx->entity.ops = &brx_entity_ops;
 	brx->entity.type = type;
+	brx->entity.codes = brx_codes;
+	brx->entity.num_codes = ARRAY_SIZE(brx_codes);
 
 	if (type == VSP1_ENTITY_BRU) {
 		num_pads = vsp1->info->num_bru_inputs + 1;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_clu.c b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
index 98645bd2a983..a16c9c941512 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
@@ -122,30 +122,20 @@ static const unsigned int clu_codes[] = {
 	MEDIA_BUS_FMT_AYUV8_1X32,
 };
 
-static int clu_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, clu_codes,
-					  ARRAY_SIZE(clu_codes));
-}
-
 static int clu_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,
-					   CLU_MIN_SIZE,
-					   CLU_MIN_SIZE, CLU_MAX_SIZE,
-					   CLU_MAX_SIZE);
+					   CLU_MIN_SIZE, CLU_MIN_SIZE,
+					   CLU_MAX_SIZE, CLU_MAX_SIZE);
 }
 
 static int clu_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, clu_codes,
-					  ARRAY_SIZE(clu_codes),
+	return vsp1_subdev_set_pad_format(subdev, sd_state, fmt,
 					  CLU_MIN_SIZE, CLU_MIN_SIZE,
 					  CLU_MAX_SIZE, CLU_MAX_SIZE);
 }
@@ -155,7 +145,7 @@ static int clu_set_format(struct v4l2_subdev *subdev,
  */
 
 static const struct v4l2_subdev_pad_ops clu_pad_ops = {
-	.enum_mbus_code = clu_enum_mbus_code,
+	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
 	.enum_frame_size = clu_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = clu_set_format,
@@ -247,6 +237,8 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device *vsp1)
 
 	clu->entity.ops = &clu_entity_ops;
 	clu->entity.type = VSP1_ENTITY_CLU;
+	clu->entity.codes = clu_codes;
+	clu->entity.num_codes = ARRAY_SIZE(clu_codes);
 
 	ret = vsp1_entity_init(vsp1, &clu->entity, "clu", 2, &clu_ops,
 			       MEDIA_ENT_F_PROC_VIDEO_LUT);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index a6680d531872..7ae1e5ab1af6 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -181,8 +181,6 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
  * @subdev: V4L2 subdevice
  * @sd_state: V4L2 subdev state
  * @code: Media bus code enumeration
- * @codes: Array of supported media bus codes
- * @ncodes: Number of supported media bus codes
  *
  * This function implements the subdev enum_mbus_code pad operation for entities
  * that do not support format conversion. It enumerates the given supported
@@ -191,16 +189,15 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
  */
 int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
-			       struct v4l2_subdev_mbus_code_enum *code,
-			       const unsigned int *codes, unsigned int ncodes)
+			       struct v4l2_subdev_mbus_code_enum *code)
 {
 	struct vsp1_entity *entity = to_vsp1_entity(subdev);
 
 	if (code->pad == 0) {
-		if (code->index >= ncodes)
+		if (code->index >= entity->num_codes)
 			return -EINVAL;
 
-		code->code = codes[code->index];
+		code->code = entity->codes[code->index];
 	} else {
 		struct v4l2_subdev_state *state;
 		struct v4l2_mbus_framefmt *format;
@@ -290,15 +287,13 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
  * @subdev: V4L2 subdevice
  * @sd_state: V4L2 subdev state
  * @fmt: V4L2 subdev format
- * @codes: Array of supported media bus codes
- * @ncodes: Number of supported media bus codes
  * @min_width: Minimum image width
  * @min_height: Minimum image height
  * @max_width: Maximum image width
  * @max_height: Maximum image height
  *
  * This function implements the subdev set_fmt pad operation for entities that
- * do not support scaling or cropping. It defaults to the first supplied media
+ * do not support scaling or cropping. It defaults to the first supported media
  * bus code if the requested code isn't supported, clamps the size to the
  * supplied minimum and maximum, and propagates the sink pad format to the
  * source pad.
@@ -306,7 +301,6 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
 int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
 			       struct v4l2_subdev_format *fmt,
-			       const unsigned int *codes, unsigned int ncodes,
 			       unsigned int min_width, unsigned int min_height,
 			       unsigned int max_width, unsigned int max_height)
 {
@@ -337,12 +331,13 @@ int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
 	 * Default to the first media bus code if the requested format is not
 	 * supported.
 	 */
-	for (i = 0; i < ncodes; ++i) {
-		if (fmt->format.code == codes[i])
+	for (i = 0; i < entity->num_codes; ++i) {
+		if (fmt->format.code == entity->codes[i])
 			break;
 	}
 
-	format->code = i < ncodes ? codes[i] : codes[0];
+	format->code = i < entity->num_codes
+		     ? entity->codes[i] : entity->codes[0];
 	format->width = clamp_t(unsigned int, fmt->format.width,
 				min_width, max_width);
 	format->height = clamp_t(unsigned int, fmt->format.height,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index b7c72d0b7f8e..13ee95402234 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -113,6 +113,9 @@ struct vsp1_entity {
 	unsigned int index;
 	const struct vsp1_route *route;
 
+	const u32 *codes;
+	unsigned int num_codes;
+
 	struct vsp1_pipeline *pipe;
 
 	struct list_head list_dev;
@@ -181,13 +184,11 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
 int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
 			       struct v4l2_subdev_format *fmt,
-			       const unsigned int *codes, unsigned int ncodes,
 			       unsigned int min_width, unsigned int min_height,
 			       unsigned int max_width, unsigned int max_height);
 int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
-			       struct v4l2_subdev_mbus_code_enum *code,
-			       const unsigned int *codes, unsigned int ncodes);
+			       struct v4l2_subdev_mbus_code_enum *code);
 int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
 				struct v4l2_subdev_state *sd_state,
 				struct v4l2_subdev_frame_size_enum *fse,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_histo.c b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
index c762202877ba..631751dbc6d3 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
@@ -167,16 +167,12 @@ static int histo_enum_mbus_code(struct v4l2_subdev *subdev,
 				struct v4l2_subdev_state *sd_state,
 				struct v4l2_subdev_mbus_code_enum *code)
 {
-	struct vsp1_histogram *histo = subdev_to_histo(subdev);
-
 	if (code->pad == HISTO_PAD_SOURCE) {
 		code->code = MEDIA_BUS_FMT_FIXED;
 		return 0;
 	}
 
-	return vsp1_subdev_enum_mbus_code(subdev, sd_state, code,
-					  histo->formats,
-					  histo->num_formats);
+	return vsp1_subdev_enum_mbus_code(subdev, sd_state, code);
 }
 
 static int histo_enum_frame_size(struct v4l2_subdev *subdev,
@@ -187,9 +183,8 @@ static int histo_enum_frame_size(struct v4l2_subdev *subdev,
 		return -EINVAL;
 
 	return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
-					   HISTO_MIN_SIZE,
-					   HISTO_MIN_SIZE, HISTO_MAX_SIZE,
-					   HISTO_MAX_SIZE);
+					   HISTO_MIN_SIZE, HISTO_MIN_SIZE,
+					   HISTO_MAX_SIZE, HISTO_MAX_SIZE);
 }
 
 static int histo_get_selection(struct v4l2_subdev *subdev,
@@ -354,8 +349,6 @@ static int histo_set_format(struct v4l2_subdev *subdev,
 			    struct v4l2_subdev_state *sd_state,
 			    struct v4l2_subdev_format *fmt)
 {
-	struct vsp1_histogram *histo = subdev_to_histo(subdev);
-
 	if (fmt->pad == HISTO_PAD_SOURCE) {
 		fmt->format.code = MEDIA_BUS_FMT_FIXED;
 		fmt->format.width = 0;
@@ -367,7 +360,6 @@ static int histo_set_format(struct v4l2_subdev *subdev,
 	}
 
 	return vsp1_subdev_set_pad_format(subdev, sd_state, fmt,
-					  histo->formats, histo->num_formats,
 					  HISTO_MIN_SIZE, HISTO_MIN_SIZE,
 					  HISTO_MAX_SIZE, HISTO_MAX_SIZE);
 }
@@ -490,8 +482,6 @@ int vsp1_histogram_init(struct vsp1_device *vsp1, struct vsp1_histogram *histo,
 {
 	int ret;
 
-	histo->formats = formats;
-	histo->num_formats = num_formats;
 	histo->data_size = data_size;
 	histo->meta_format = meta_format;
 
@@ -506,6 +496,8 @@ int vsp1_histogram_init(struct vsp1_device *vsp1, struct vsp1_histogram *histo,
 	/* Initialize the VSP entity... */
 	histo->entity.ops = ops;
 	histo->entity.type = type;
+	histo->entity.codes = formats;
+	histo->entity.num_codes = num_formats;
 
 	ret = vsp1_entity_init(vsp1, &histo->entity, name, 2, &histo_ops,
 			       MEDIA_ENT_F_PROC_VIDEO_STATISTICS);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_histo.h b/drivers/media/platform/renesas/vsp1/vsp1_histo.h
index 06f029846244..227db810da52 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_histo.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_histo.h
@@ -36,8 +36,6 @@ struct vsp1_histogram {
 	struct video_device video;
 	struct media_pad pad;
 
-	const u32 *formats;
-	unsigned int num_formats;
 	size_t data_size;
 	u32 meta_format;
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
index 1fcd1967d3b2..927dc185b8f7 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
@@ -34,6 +34,11 @@ static inline void vsp1_hsit_write(struct vsp1_hsit *hsit,
  * V4L2 Subdevice Operations
  */
 
+static const unsigned int hsit_codes[] = {
+	MEDIA_BUS_FMT_ARGB8888_1X32,
+	MEDIA_BUS_FMT_AHSV8888_1X32,
+};
+
 static int hsit_enum_mbus_code(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
 			       struct v4l2_subdev_mbus_code_enum *code)
@@ -57,9 +62,8 @@ static int hsit_enum_frame_size(struct v4l2_subdev *subdev,
 				struct v4l2_subdev_frame_size_enum *fse)
 {
 	return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
-					   HSIT_MIN_SIZE,
-					   HSIT_MIN_SIZE, HSIT_MAX_SIZE,
-					   HSIT_MAX_SIZE);
+					   HSIT_MIN_SIZE, HSIT_MIN_SIZE,
+					   HSIT_MAX_SIZE, HSIT_MAX_SIZE);
 }
 
 static int hsit_set_format(struct v4l2_subdev *subdev,
@@ -175,6 +179,9 @@ struct vsp1_hsit *vsp1_hsit_create(struct vsp1_device *vsp1, bool inverse)
 	else
 		hsit->entity.type = VSP1_ENTITY_HST;
 
+	hsit->entity.codes = hsit_codes;
+	hsit->entity.num_codes = ARRAY_SIZE(hsit_codes);
+
 	ret = vsp1_entity_init(vsp1, &hsit->entity, inverse ? "hsi" : "hst",
 			       2, &hsit_ops,
 			       MEDIA_ENT_F_PROC_VIDEO_PIXEL_ENC_CONV);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_iif.c b/drivers/media/platform/renesas/vsp1/vsp1_iif.c
index 5dd62bebbe8c..018055fa1f78 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_iif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_iif.c
@@ -36,14 +36,6 @@ static const unsigned int iif_codes[] = {
 	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)
@@ -64,7 +56,7 @@ static int iif_set_format(struct v4l2_subdev *subdev,
 }
 
 static const struct v4l2_subdev_pad_ops iif_pad_ops = {
-	.enum_mbus_code = iif_enum_mbus_code,
+	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
 	.enum_frame_size = iif_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = iif_set_format,
@@ -106,6 +98,8 @@ struct vsp1_iif *vsp1_iif_create(struct vsp1_device *vsp1)
 
 	iif->entity.ops = &iif_entity_ops;
 	iif->entity.type = VSP1_ENTITY_IIF;
+	iif->entity.codes = iif_codes;
+	iif->entity.num_codes = ARRAY_SIZE(iif_codes);
 
 	/*
 	 * The IIF is never exposed to userspace, but media entity registration
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
index b3d83d1c5306..6c1cbe2d8524 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
@@ -39,36 +39,26 @@ static const unsigned int lif_codes[] = {
 	MEDIA_BUS_FMT_AYUV8_1X32,
 };
 
-static int lif_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, lif_codes,
-					  ARRAY_SIZE(lif_codes));
-}
-
 static int lif_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,
-					   LIF_MIN_SIZE,
-					   LIF_MIN_SIZE, LIF_MAX_SIZE,
-					   LIF_MAX_SIZE);
+					   LIF_MIN_SIZE, LIF_MIN_SIZE,
+					   LIF_MAX_SIZE, LIF_MAX_SIZE);
 }
 
 static int lif_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, lif_codes,
-					  ARRAY_SIZE(lif_codes),
+	return vsp1_subdev_set_pad_format(subdev, sd_state, fmt,
 					  LIF_MIN_SIZE, LIF_MIN_SIZE,
 					  LIF_MAX_SIZE, LIF_MAX_SIZE);
 }
 
 static const struct v4l2_subdev_pad_ops lif_pad_ops = {
-	.enum_mbus_code = lif_enum_mbus_code,
+	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
 	.enum_frame_size = lif_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = lif_set_format,
@@ -162,6 +152,8 @@ struct vsp1_lif *vsp1_lif_create(struct vsp1_device *vsp1, unsigned int index)
 	lif->entity.ops = &lif_entity_ops;
 	lif->entity.type = VSP1_ENTITY_LIF;
 	lif->entity.index = index;
+	lif->entity.codes = lif_codes;
+	lif->entity.num_codes = ARRAY_SIZE(lif_codes);
 
 	/*
 	 * The LIF is never exposed to userspace, but media entity registration
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lut.c b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
index dd264e6532e0..46c79cdccd69 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
@@ -98,30 +98,20 @@ static const unsigned int lut_codes[] = {
 	MEDIA_BUS_FMT_AYUV8_1X32,
 };
 
-static int lut_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, lut_codes,
-					  ARRAY_SIZE(lut_codes));
-}
-
 static int lut_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,
-					   LUT_MIN_SIZE,
-					   LUT_MIN_SIZE, LUT_MAX_SIZE,
-					   LUT_MAX_SIZE);
+					   LUT_MIN_SIZE, LUT_MIN_SIZE,
+					   LUT_MAX_SIZE, LUT_MAX_SIZE);
 }
 
 static int lut_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, lut_codes,
-					  ARRAY_SIZE(lut_codes),
+	return vsp1_subdev_set_pad_format(subdev, sd_state, fmt,
 					  LUT_MIN_SIZE, LUT_MIN_SIZE,
 					  LUT_MAX_SIZE, LUT_MAX_SIZE);
 }
@@ -131,7 +121,7 @@ static int lut_set_format(struct v4l2_subdev *subdev,
  */
 
 static const struct v4l2_subdev_pad_ops lut_pad_ops = {
-	.enum_mbus_code = lut_enum_mbus_code,
+	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
 	.enum_frame_size = lut_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = lut_set_format,
@@ -208,6 +198,8 @@ struct vsp1_lut *vsp1_lut_create(struct vsp1_device *vsp1)
 
 	lut->entity.ops = &lut_entity_ops;
 	lut->entity.type = VSP1_ENTITY_LUT;
+	lut->entity.codes = lut_codes;
+	lut->entity.num_codes = ARRAY_SIZE(lut_codes);
 
 	ret = vsp1_entity_init(vsp1, &lut->entity, "lut", 2, &lut_ops,
 			       MEDIA_ENT_F_PROC_VIDEO_LUT);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index 9c8085d5d306..304a2f618777 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -21,20 +21,20 @@
  * V4L2 Subdevice Operations
  */
 
+static const unsigned int rwpf_codes[] = {
+	MEDIA_BUS_FMT_ARGB8888_1X32,
+	MEDIA_BUS_FMT_AHSV8888_1X32,
+	MEDIA_BUS_FMT_AYUV8_1X32,
+};
+
 static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev,
 				    struct v4l2_subdev_state *sd_state,
 				    struct v4l2_subdev_mbus_code_enum *code)
 {
-	static const unsigned int codes[] = {
-		MEDIA_BUS_FMT_ARGB8888_1X32,
-		MEDIA_BUS_FMT_AHSV8888_1X32,
-		MEDIA_BUS_FMT_AYUV8_1X32,
-	};
-
-	if (code->index >= ARRAY_SIZE(codes))
+	if (code->index >= ARRAY_SIZE(rwpf_codes))
 		return -EINVAL;
 
-	code->code = codes[code->index];
+	code->code = rwpf_codes[code->index];
 
 	if (code->pad == RWPF_PAD_SOURCE &&
 	    code->code == MEDIA_BUS_FMT_AYUV8_1X32)
@@ -51,9 +51,8 @@ static int vsp1_rwpf_enum_frame_size(struct v4l2_subdev *subdev,
 	struct vsp1_rwpf *rwpf = to_rwpf(subdev);
 
 	return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
-					   RWPF_MIN_WIDTH,
-					   RWPF_MIN_HEIGHT, rwpf->max_width,
-					   rwpf->max_height);
+					   RWPF_MIN_WIDTH, RWPF_MIN_HEIGHT,
+					   rwpf->max_width, rwpf->max_height);
 }
 
 static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
@@ -311,6 +310,9 @@ static const struct v4l2_ctrl_ops vsp1_rwpf_ctrl_ops = {
 
 int vsp1_rwpf_init_ctrls(struct vsp1_rwpf *rwpf, unsigned int ncontrols)
 {
+	rwpf->entity.codes = rwpf_codes;
+	rwpf->entity.num_codes = ARRAY_SIZE(rwpf_codes);
+
 	v4l2_ctrl_handler_init(&rwpf->ctrls, ncontrols + 1);
 	v4l2_ctrl_new_std(&rwpf->ctrls, &vsp1_rwpf_ctrl_ops,
 			  V4L2_CID_ALPHA_COMPONENT, 0, 255, 1, 255);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
index bba2872afaf2..8e587efc0dc2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -106,18 +106,10 @@ static const struct v4l2_ctrl_config sru_intensity_control = {
  * V4L2 Subdevice Operations
  */
 
-static int sru_enum_mbus_code(struct v4l2_subdev *subdev,
-			      struct v4l2_subdev_state *sd_state,
-			      struct v4l2_subdev_mbus_code_enum *code)
-{
-	static const unsigned int codes[] = {
-		MEDIA_BUS_FMT_ARGB8888_1X32,
-		MEDIA_BUS_FMT_AYUV8_1X32,
-	};
-
-	return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, codes,
-					  ARRAY_SIZE(codes));
-}
+static const unsigned int sru_codes[] = {
+	MEDIA_BUS_FMT_ARGB8888_1X32,
+	MEDIA_BUS_FMT_AYUV8_1X32,
+};
 
 static int sru_enum_frame_size(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
@@ -257,7 +249,7 @@ static int sru_set_format(struct v4l2_subdev *subdev,
 }
 
 static const struct v4l2_subdev_pad_ops sru_pad_ops = {
-	.enum_mbus_code = sru_enum_mbus_code,
+	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
 	.enum_frame_size = sru_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = sru_set_format,
@@ -370,6 +362,8 @@ struct vsp1_sru *vsp1_sru_create(struct vsp1_device *vsp1)
 
 	sru->entity.ops = &sru_entity_ops;
 	sru->entity.type = VSP1_ENTITY_SRU;
+	sru->entity.codes = sru_codes;
+	sru->entity.num_codes = ARRAY_SIZE(sru_codes);
 
 	ret = vsp1_entity_init(vsp1, &sru->entity, "sru", 2, &sru_ops,
 			       MEDIA_ENT_F_PROC_VIDEO_SCALER);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index 2db473b6f83c..928b09e20add 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -111,18 +111,10 @@ static unsigned int uds_compute_ratio(unsigned int input, unsigned int output)
  * V4L2 Subdevice Pad Operations
  */
 
-static int uds_enum_mbus_code(struct v4l2_subdev *subdev,
-			      struct v4l2_subdev_state *sd_state,
-			      struct v4l2_subdev_mbus_code_enum *code)
-{
-	static const unsigned int codes[] = {
-		MEDIA_BUS_FMT_ARGB8888_1X32,
-		MEDIA_BUS_FMT_AYUV8_1X32,
-	};
-
-	return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, codes,
-					  ARRAY_SIZE(codes));
-}
+static const unsigned int uds_codes[] = {
+	MEDIA_BUS_FMT_ARGB8888_1X32,
+	MEDIA_BUS_FMT_AYUV8_1X32,
+};
 
 static int uds_enum_frame_size(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
@@ -244,7 +236,7 @@ static int uds_set_format(struct v4l2_subdev *subdev,
  */
 
 static const struct v4l2_subdev_pad_ops uds_pad_ops = {
-	.enum_mbus_code = uds_enum_mbus_code,
+	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
 	.enum_frame_size = uds_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = uds_set_format,
@@ -410,6 +402,8 @@ struct vsp1_uds *vsp1_uds_create(struct vsp1_device *vsp1, unsigned int index)
 	uds->entity.ops = &uds_entity_ops;
 	uds->entity.type = VSP1_ENTITY_UDS;
 	uds->entity.index = index;
+	uds->entity.codes = uds_codes;
+	uds->entity.num_codes = ARRAY_SIZE(uds_codes);
 
 	sprintf(name, "uds.%u", index);
 	ret = vsp1_entity_init(vsp1, &uds->entity, name, 2, &uds_ops,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uif.c b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
index edaf28b544d2..e1bb6c709721 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
@@ -53,30 +53,20 @@ static const unsigned int uif_codes[] = {
 	MEDIA_BUS_FMT_AYUV8_1X32,
 };
 
-static int uif_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, uif_codes,
-					  ARRAY_SIZE(uif_codes));
-}
-
 static int uif_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,
-					   UIF_MIN_SIZE,
-					   UIF_MIN_SIZE, UIF_MAX_SIZE,
-					   UIF_MAX_SIZE);
+					   UIF_MIN_SIZE, UIF_MIN_SIZE,
+					   UIF_MAX_SIZE, UIF_MAX_SIZE);
 }
 
 static int uif_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, uif_codes,
-					  ARRAY_SIZE(uif_codes),
+	return vsp1_subdev_set_pad_format(subdev, sd_state, fmt,
 					  UIF_MIN_SIZE, UIF_MIN_SIZE,
 					  UIF_MAX_SIZE, UIF_MAX_SIZE);
 }
@@ -171,7 +161,7 @@ static int uif_set_selection(struct v4l2_subdev *subdev,
  */
 
 static const struct v4l2_subdev_pad_ops uif_pad_ops = {
-	.enum_mbus_code = uif_enum_mbus_code,
+	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
 	.enum_frame_size = uif_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = uif_set_format,
@@ -250,6 +240,8 @@ struct vsp1_uif *vsp1_uif_create(struct vsp1_device *vsp1, unsigned int index)
 	uif->entity.ops = &uif_entity_ops;
 	uif->entity.type = VSP1_ENTITY_UIF;
 	uif->entity.index = index;
+	uif->entity.codes = uif_codes;
+	uif->entity.num_codes = ARRAY_SIZE(uif_codes);
 
 	/* The datasheet names the two UIF instances UIF4 and UIF5. */
 	sprintf(name, "uif.%u", index + 4);
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/8] media: renesas: vsp1: Store size limits in vsp1_entity
  2025-07-03 22:38 [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 1/8] media: renesas: vsp1: Store supported media bus codes in vsp1_entity Laurent Pinchart
@ 2025-07-03 22:38 ` Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 3/8] media: renesas: vsp1: Fix code checks in frame size enumeration Laurent Pinchart
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-07-03 22:38 UTC (permalink / raw)
  To: linux-media
  Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen

Most entities use the vsp1_subdev_enum_frame_size() and
vsp1_subdev_set_pad_format() helper functions to implement the
corresponding subdev operations. Both helpers are given the minimum and
maximum sizes supported by the entity as arguments, requiring each
entity to implement a wrapper.

Replace the function arguments with storing the size limits in the
vsp1_entity structure. This allows dropping most of the
.enum_frame_size() and .set_fmt() wrappers in entities.

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

- Initialize the minimum width and height for RPF and WPF in the
  corresponding files
- Address IIF entity
---
 .../media/platform/renesas/vsp1/vsp1_brx.c    |  4 +++
 .../media/platform/renesas/vsp1/vsp1_clu.c    | 32 ++++---------------
 .../media/platform/renesas/vsp1/vsp1_entity.c | 31 ++++++------------
 .../media/platform/renesas/vsp1/vsp1_entity.h | 12 +++----
 .../media/platform/renesas/vsp1/vsp1_histo.c  | 12 +++----
 .../media/platform/renesas/vsp1/vsp1_hsit.c   | 15 +++------
 .../media/platform/renesas/vsp1/vsp1_iif.c    | 27 ++++------------
 .../media/platform/renesas/vsp1/vsp1_lif.c    | 26 ++++-----------
 .../media/platform/renesas/vsp1/vsp1_lut.c    | 32 ++++---------------
 .../media/platform/renesas/vsp1/vsp1_rpf.c    |  7 ++--
 .../media/platform/renesas/vsp1/vsp1_rwpf.c   | 20 ++----------
 .../media/platform/renesas/vsp1/vsp1_rwpf.h   |  6 ++--
 .../media/platform/renesas/vsp1/vsp1_sru.c    |  4 +++
 .../media/platform/renesas/vsp1/vsp1_uds.c    |  4 +++
 .../media/platform/renesas/vsp1/vsp1_uif.c    | 26 ++++-----------
 .../media/platform/renesas/vsp1/vsp1_wpf.c    | 13 +++++---
 16 files changed, 88 insertions(+), 183 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index 3890adc8073e..dd651cef93e4 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -410,6 +410,10 @@ struct vsp1_brx *vsp1_brx_create(struct vsp1_device *vsp1,
 	brx->entity.type = type;
 	brx->entity.codes = brx_codes;
 	brx->entity.num_codes = ARRAY_SIZE(brx_codes);
+	brx->entity.min_width = BRX_MIN_SIZE;
+	brx->entity.max_width = BRX_MAX_SIZE;
+	brx->entity.min_height = BRX_MIN_SIZE;
+	brx->entity.max_height = BRX_MAX_SIZE;
 
 	if (type == VSP1_ENTITY_BRU) {
 		num_pads = vsp1->info->num_bru_inputs + 1;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_clu.c b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
index a16c9c941512..a56c038a2c71 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
@@ -113,7 +113,7 @@ static const struct v4l2_ctrl_config clu_mode_control = {
 };
 
 /* -----------------------------------------------------------------------------
- * V4L2 Subdevice Pad Operations
+ * V4L2 Subdevice Operations
  */
 
 static const unsigned int clu_codes[] = {
@@ -122,33 +122,11 @@ static const unsigned int clu_codes[] = {
 	MEDIA_BUS_FMT_AYUV8_1X32,
 };
 
-static int clu_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,
-					   CLU_MIN_SIZE, CLU_MIN_SIZE,
-					   CLU_MAX_SIZE, CLU_MAX_SIZE);
-}
-
-static int clu_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,
-					  CLU_MIN_SIZE, CLU_MIN_SIZE,
-					  CLU_MAX_SIZE, CLU_MAX_SIZE);
-}
-
-/* -----------------------------------------------------------------------------
- * V4L2 Subdevice Operations
- */
-
 static const struct v4l2_subdev_pad_ops clu_pad_ops = {
 	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
-	.enum_frame_size = clu_enum_frame_size,
+	.enum_frame_size = vsp1_subdev_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
-	.set_fmt = clu_set_format,
+	.set_fmt = vsp1_subdev_set_pad_format,
 };
 
 static const struct v4l2_subdev_ops clu_ops = {
@@ -239,6 +217,10 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device *vsp1)
 	clu->entity.type = VSP1_ENTITY_CLU;
 	clu->entity.codes = clu_codes;
 	clu->entity.num_codes = ARRAY_SIZE(clu_codes);
+	clu->entity.min_width = CLU_MIN_SIZE;
+	clu->entity.min_height = CLU_MIN_SIZE;
+	clu->entity.max_width = CLU_MAX_SIZE;
+	clu->entity.max_height = CLU_MAX_SIZE;
 
 	ret = vsp1_entity_init(vsp1, &clu->entity, "clu", 2, &clu_ops,
 			       MEDIA_ENT_F_PROC_VIDEO_LUT);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 7ae1e5ab1af6..04b7ae6fb935 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -227,10 +227,6 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
  * @subdev: V4L2 subdevice
  * @sd_state: V4L2 subdev state
  * @fse: Frame size enumeration
- * @min_width: Minimum image width
- * @min_height: Minimum image height
- * @max_width: Maximum image width
- * @max_height: Maximum image height
  *
  * This function implements the subdev enum_frame_size pad operation for
  * entities that do not support scaling or cropping. It reports the given
@@ -239,9 +235,7 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
  */
 int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
 				struct v4l2_subdev_state *sd_state,
-				struct v4l2_subdev_frame_size_enum *fse,
-				unsigned int min_width, unsigned int min_height,
-				unsigned int max_width, unsigned int max_height)
+				struct v4l2_subdev_frame_size_enum *fse)
 {
 	struct vsp1_entity *entity = to_vsp1_entity(subdev);
 	struct v4l2_subdev_state *state;
@@ -262,10 +256,10 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
 	}
 
 	if (fse->pad == 0) {
-		fse->min_width = min_width;
-		fse->max_width = max_width;
-		fse->min_height = min_height;
-		fse->max_height = max_height;
+		fse->min_width = entity->min_width;
+		fse->max_width = entity->max_width;
+		fse->min_height = entity->min_height;
+		fse->max_height = entity->max_height;
 	} else {
 		/*
 		 * The size on the source pad are fixed and always identical to
@@ -287,22 +281,15 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
  * @subdev: V4L2 subdevice
  * @sd_state: V4L2 subdev state
  * @fmt: V4L2 subdev format
- * @min_width: Minimum image width
- * @min_height: Minimum image height
- * @max_width: Maximum image width
- * @max_height: Maximum image height
  *
  * This function implements the subdev set_fmt pad operation for entities that
  * do not support scaling or cropping. It defaults to the first supported media
  * bus code if the requested code isn't supported, clamps the size to the
- * supplied minimum and maximum, and propagates the sink pad format to the
- * source pad.
+ * entity's limits, and propagates the sink pad format to the source pad.
  */
 int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
-			       struct v4l2_subdev_format *fmt,
-			       unsigned int min_width, unsigned int min_height,
-			       unsigned int max_width, unsigned int max_height)
+			       struct v4l2_subdev_format *fmt)
 {
 	struct vsp1_entity *entity = to_vsp1_entity(subdev);
 	struct v4l2_subdev_state *state;
@@ -339,9 +326,9 @@ int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
 	format->code = i < entity->num_codes
 		     ? entity->codes[i] : entity->codes[0];
 	format->width = clamp_t(unsigned int, fmt->format.width,
-				min_width, max_width);
+				entity->min_width, entity->max_width);
 	format->height = clamp_t(unsigned int, fmt->format.height,
-				 min_height, max_height);
+				 entity->min_height, entity->max_height);
 	format->field = V4L2_FIELD_NONE;
 
 	format->colorspace = fmt->format.colorspace;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index 13ee95402234..88daf83cd1ab 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -115,6 +115,10 @@ struct vsp1_entity {
 
 	const u32 *codes;
 	unsigned int num_codes;
+	unsigned int min_width;
+	unsigned int min_height;
+	unsigned int max_width;
+	unsigned int max_height;
 
 	struct vsp1_pipeline *pipe;
 
@@ -183,16 +187,12 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_format *fmt);
 int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
-			       struct v4l2_subdev_format *fmt,
-			       unsigned int min_width, unsigned int min_height,
-			       unsigned int max_width, unsigned int max_height);
+			       struct v4l2_subdev_format *fmt);
 int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_state *sd_state,
 			       struct v4l2_subdev_mbus_code_enum *code);
 int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
 				struct v4l2_subdev_state *sd_state,
-				struct v4l2_subdev_frame_size_enum *fse,
-				unsigned int min_w, unsigned int min_h,
-				unsigned int max_w, unsigned int max_h);
+				struct v4l2_subdev_frame_size_enum *fse);
 
 #endif /* __VSP1_ENTITY_H__ */
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_histo.c b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
index 631751dbc6d3..a1b3671a0873 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
@@ -182,9 +182,7 @@ static int histo_enum_frame_size(struct v4l2_subdev *subdev,
 	if (fse->pad != HISTO_PAD_SINK)
 		return -EINVAL;
 
-	return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
-					   HISTO_MIN_SIZE, HISTO_MIN_SIZE,
-					   HISTO_MAX_SIZE, HISTO_MAX_SIZE);
+	return vsp1_subdev_enum_frame_size(subdev, sd_state, fse);
 }
 
 static int histo_get_selection(struct v4l2_subdev *subdev,
@@ -359,9 +357,7 @@ static int histo_set_format(struct v4l2_subdev *subdev,
 		return 0;
 	}
 
-	return vsp1_subdev_set_pad_format(subdev, sd_state, fmt,
-					  HISTO_MIN_SIZE, HISTO_MIN_SIZE,
-					  HISTO_MAX_SIZE, HISTO_MAX_SIZE);
+	return vsp1_subdev_set_pad_format(subdev, sd_state, fmt);
 }
 
 static const struct v4l2_subdev_pad_ops histo_pad_ops = {
@@ -498,6 +494,10 @@ int vsp1_histogram_init(struct vsp1_device *vsp1, struct vsp1_histogram *histo,
 	histo->entity.type = type;
 	histo->entity.codes = formats;
 	histo->entity.num_codes = num_formats;
+	histo->entity.min_width = HISTO_MIN_SIZE;
+	histo->entity.min_height = HISTO_MIN_SIZE;
+	histo->entity.max_width = HISTO_MAX_SIZE;
+	histo->entity.max_height = HISTO_MAX_SIZE;
 
 	ret = vsp1_entity_init(vsp1, &histo->entity, name, 2, &histo_ops,
 			       MEDIA_ENT_F_PROC_VIDEO_STATISTICS);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
index 927dc185b8f7..8260934db789 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
@@ -57,15 +57,6 @@ static int hsit_enum_mbus_code(struct v4l2_subdev *subdev,
 	return 0;
 }
 
-static int hsit_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,
-					   HSIT_MIN_SIZE, HSIT_MIN_SIZE,
-					   HSIT_MAX_SIZE, HSIT_MAX_SIZE);
-}
-
 static int hsit_set_format(struct v4l2_subdev *subdev,
 			   struct v4l2_subdev_state *sd_state,
 			   struct v4l2_subdev_format *fmt)
@@ -126,7 +117,7 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
 
 static const struct v4l2_subdev_pad_ops hsit_pad_ops = {
 	.enum_mbus_code = hsit_enum_mbus_code,
-	.enum_frame_size = hsit_enum_frame_size,
+	.enum_frame_size = vsp1_subdev_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = hsit_set_format,
 };
@@ -181,6 +172,10 @@ struct vsp1_hsit *vsp1_hsit_create(struct vsp1_device *vsp1, bool inverse)
 
 	hsit->entity.codes = hsit_codes;
 	hsit->entity.num_codes = ARRAY_SIZE(hsit_codes);
+	hsit->entity.min_width = HSIT_MIN_SIZE;
+	hsit->entity.min_height = HSIT_MIN_SIZE;
+	hsit->entity.max_width = HSIT_MAX_SIZE;
+	hsit->entity.max_height = HSIT_MAX_SIZE;
 
 	ret = vsp1_entity_init(vsp1, &hsit->entity, inverse ? "hsi" : "hst",
 			       2, &hsit_ops,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_iif.c b/drivers/media/platform/renesas/vsp1/vsp1_iif.c
index 018055fa1f78..d44c04e140bc 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_iif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_iif.c
@@ -36,30 +36,11 @@ static const unsigned int iif_codes[] = {
 	MEDIA_BUS_FMT_METADATA_FIXED
 };
 
-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 = vsp1_subdev_enum_mbus_code,
-	.enum_frame_size = iif_enum_frame_size,
+	.enum_frame_size = vsp1_subdev_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
-	.set_fmt = iif_set_format,
+	.set_fmt = vsp1_subdev_set_pad_format,
 };
 
 static const struct v4l2_subdev_ops iif_ops = {
@@ -100,6 +81,10 @@ struct vsp1_iif *vsp1_iif_create(struct vsp1_device *vsp1)
 	iif->entity.type = VSP1_ENTITY_IIF;
 	iif->entity.codes = iif_codes;
 	iif->entity.num_codes = ARRAY_SIZE(iif_codes);
+	iif->entity.min_width = IIF_MIN_WIDTH;
+	iif->entity.min_height = IIF_MIN_HEIGHT;
+	iif->entity.max_width = IIF_MAX_WIDTH;
+	iif->entity.max_height = IIF_MAX_HEIGHT;
 
 	/*
 	 * The IIF is never exposed to userspace, but media entity registration
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
index 6c1cbe2d8524..1ebb88b3e4c9 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
@@ -39,29 +39,11 @@ static const unsigned int lif_codes[] = {
 	MEDIA_BUS_FMT_AYUV8_1X32,
 };
 
-static int lif_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,
-					   LIF_MIN_SIZE, LIF_MIN_SIZE,
-					   LIF_MAX_SIZE, LIF_MAX_SIZE);
-}
-
-static int lif_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,
-					  LIF_MIN_SIZE, LIF_MIN_SIZE,
-					  LIF_MAX_SIZE, LIF_MAX_SIZE);
-}
-
 static const struct v4l2_subdev_pad_ops lif_pad_ops = {
 	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
-	.enum_frame_size = lif_enum_frame_size,
+	.enum_frame_size = vsp1_subdev_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
-	.set_fmt = lif_set_format,
+	.set_fmt = vsp1_subdev_set_pad_format,
 };
 
 static const struct v4l2_subdev_ops lif_ops = {
@@ -154,6 +136,10 @@ struct vsp1_lif *vsp1_lif_create(struct vsp1_device *vsp1, unsigned int index)
 	lif->entity.index = index;
 	lif->entity.codes = lif_codes;
 	lif->entity.num_codes = ARRAY_SIZE(lif_codes);
+	lif->entity.min_width = LIF_MIN_SIZE;
+	lif->entity.min_height = LIF_MIN_SIZE;
+	lif->entity.max_width = LIF_MAX_SIZE;
+	lif->entity.max_height = LIF_MAX_SIZE;
 
 	/*
 	 * The LIF is never exposed to userspace, but media entity registration
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lut.c b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
index 46c79cdccd69..2ec4d596465d 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
@@ -89,7 +89,7 @@ static const struct v4l2_ctrl_config lut_table_control = {
 };
 
 /* -----------------------------------------------------------------------------
- * V4L2 Subdevice Pad Operations
+ * V4L2 Subdevice Operations
  */
 
 static const unsigned int lut_codes[] = {
@@ -98,33 +98,11 @@ static const unsigned int lut_codes[] = {
 	MEDIA_BUS_FMT_AYUV8_1X32,
 };
 
-static int lut_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,
-					   LUT_MIN_SIZE, LUT_MIN_SIZE,
-					   LUT_MAX_SIZE, LUT_MAX_SIZE);
-}
-
-static int lut_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,
-					  LUT_MIN_SIZE, LUT_MIN_SIZE,
-					  LUT_MAX_SIZE, LUT_MAX_SIZE);
-}
-
-/* -----------------------------------------------------------------------------
- * V4L2 Subdevice Operations
- */
-
 static const struct v4l2_subdev_pad_ops lut_pad_ops = {
 	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
-	.enum_frame_size = lut_enum_frame_size,
+	.enum_frame_size = vsp1_subdev_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
-	.set_fmt = lut_set_format,
+	.set_fmt = vsp1_subdev_set_pad_format,
 };
 
 static const struct v4l2_subdev_ops lut_ops = {
@@ -200,6 +178,10 @@ struct vsp1_lut *vsp1_lut_create(struct vsp1_device *vsp1)
 	lut->entity.type = VSP1_ENTITY_LUT;
 	lut->entity.codes = lut_codes;
 	lut->entity.num_codes = ARRAY_SIZE(lut_codes);
+	lut->entity.min_width = LUT_MIN_SIZE;
+	lut->entity.min_height = LUT_MIN_SIZE;
+	lut->entity.max_width = LUT_MAX_SIZE;
+	lut->entity.max_height = LUT_MAX_SIZE;
 
 	ret = vsp1_entity_init(vsp1, &lut->entity, "lut", 2, &lut_ops,
 			       MEDIA_ENT_F_PROC_VIDEO_LUT);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 811f2b7c5cc5..34b9095c9011 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -425,12 +425,13 @@ struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device *vsp1, unsigned int index)
 	if (rpf == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	rpf->max_width = RPF_MAX_WIDTH;
-	rpf->max_height = RPF_MAX_HEIGHT;
-
 	rpf->entity.ops = &rpf_entity_ops;
 	rpf->entity.type = VSP1_ENTITY_RPF;
 	rpf->entity.index = index;
+	rpf->entity.min_width = RWPF_MIN_WIDTH;
+	rpf->entity.min_height = RWPF_MIN_HEIGHT;
+	rpf->entity.max_width = RPF_MAX_WIDTH;
+	rpf->entity.max_height = RPF_MAX_HEIGHT;
 
 	sprintf(name, "rpf.%u", index);
 	ret = vsp1_entity_init(vsp1, &rpf->entity, name, 2, &vsp1_rwpf_subdev_ops,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index 304a2f618777..56464875a6c5 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -14,9 +14,6 @@
 #include "vsp1_rwpf.h"
 #include "vsp1_video.h"
 
-#define RWPF_MIN_WIDTH				1
-#define RWPF_MIN_HEIGHT				1
-
 /* -----------------------------------------------------------------------------
  * V4L2 Subdevice Operations
  */
@@ -44,17 +41,6 @@ static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev,
 	return 0;
 }
 
-static int vsp1_rwpf_enum_frame_size(struct v4l2_subdev *subdev,
-				     struct v4l2_subdev_state *sd_state,
-				     struct v4l2_subdev_frame_size_enum *fse)
-{
-	struct vsp1_rwpf *rwpf = to_rwpf(subdev);
-
-	return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
-					   RWPF_MIN_WIDTH, RWPF_MIN_HEIGHT,
-					   rwpf->max_width, rwpf->max_height);
-}
-
 static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
 				struct v4l2_subdev_state *sd_state,
 				struct v4l2_subdev_format *fmt)
@@ -125,9 +111,9 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
 
 	format->code = fmt->format.code;
 	format->width = clamp_t(unsigned int, fmt->format.width,
-				RWPF_MIN_WIDTH, rwpf->max_width);
+				RWPF_MIN_WIDTH, rwpf->entity.max_width);
 	format->height = clamp_t(unsigned int, fmt->format.height,
-				 RWPF_MIN_HEIGHT, rwpf->max_height);
+				 RWPF_MIN_HEIGHT, rwpf->entity.max_height);
 	format->field = V4L2_FIELD_NONE;
 
 	format->colorspace = fmt->format.colorspace;
@@ -275,7 +261,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
 
 static const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = {
 	.enum_mbus_code = vsp1_rwpf_enum_mbus_code,
-	.enum_frame_size = vsp1_rwpf_enum_frame_size,
+	.enum_frame_size = vsp1_subdev_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = vsp1_rwpf_set_format,
 	.get_selection = vsp1_rwpf_get_selection,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
index 5ac9f0a6fafc..89c1c8e8bb6d 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
@@ -21,6 +21,9 @@
 #define RWPF_PAD_SINK				0
 #define RWPF_PAD_SOURCE				1
 
+#define RWPF_MIN_WIDTH				1
+#define RWPF_MIN_HEIGHT				1
+
 struct v4l2_ctrl;
 struct vsp1_dl_manager;
 struct vsp1_rwpf;
@@ -36,9 +39,6 @@ struct vsp1_rwpf {
 
 	struct vsp1_video *video;
 
-	unsigned int max_width;
-	unsigned int max_height;
-
 	struct v4l2_pix_format_mplane format;
 	const struct vsp1_format_info *fmtinfo;
 	unsigned int brx_input;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
index 8e587efc0dc2..1dc34e6a510d 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -364,6 +364,10 @@ struct vsp1_sru *vsp1_sru_create(struct vsp1_device *vsp1)
 	sru->entity.type = VSP1_ENTITY_SRU;
 	sru->entity.codes = sru_codes;
 	sru->entity.num_codes = ARRAY_SIZE(sru_codes);
+	sru->entity.min_width = SRU_MIN_SIZE;
+	sru->entity.max_width = SRU_MAX_SIZE;
+	sru->entity.min_height = SRU_MIN_SIZE;
+	sru->entity.max_height = SRU_MAX_SIZE;
 
 	ret = vsp1_entity_init(vsp1, &sru->entity, "sru", 2, &sru_ops,
 			       MEDIA_ENT_F_PROC_VIDEO_SCALER);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index 928b09e20add..8006d49ffbea 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -404,6 +404,10 @@ struct vsp1_uds *vsp1_uds_create(struct vsp1_device *vsp1, unsigned int index)
 	uds->entity.index = index;
 	uds->entity.codes = uds_codes;
 	uds->entity.num_codes = ARRAY_SIZE(uds_codes);
+	uds->entity.min_width = UDS_MIN_SIZE;
+	uds->entity.max_width = UDS_MAX_SIZE;
+	uds->entity.min_height = UDS_MIN_SIZE;
+	uds->entity.max_height = UDS_MAX_SIZE;
 
 	sprintf(name, "uds.%u", index);
 	ret = vsp1_entity_init(vsp1, &uds->entity, name, 2, &uds_ops,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uif.c b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
index e1bb6c709721..3aefe5c9d421 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
@@ -53,24 +53,6 @@ static const unsigned int uif_codes[] = {
 	MEDIA_BUS_FMT_AYUV8_1X32,
 };
 
-static int uif_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,
-					   UIF_MIN_SIZE, UIF_MIN_SIZE,
-					   UIF_MAX_SIZE, UIF_MAX_SIZE);
-}
-
-static int uif_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,
-					  UIF_MIN_SIZE, UIF_MIN_SIZE,
-					  UIF_MAX_SIZE, UIF_MAX_SIZE);
-}
-
 static int uif_get_selection(struct v4l2_subdev *subdev,
 			     struct v4l2_subdev_state *sd_state,
 			     struct v4l2_subdev_selection *sel)
@@ -162,9 +144,9 @@ static int uif_set_selection(struct v4l2_subdev *subdev,
 
 static const struct v4l2_subdev_pad_ops uif_pad_ops = {
 	.enum_mbus_code = vsp1_subdev_enum_mbus_code,
-	.enum_frame_size = uif_enum_frame_size,
+	.enum_frame_size = vsp1_subdev_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
-	.set_fmt = uif_set_format,
+	.set_fmt = vsp1_subdev_set_pad_format,
 	.get_selection = uif_get_selection,
 	.set_selection = uif_set_selection,
 };
@@ -242,6 +224,10 @@ struct vsp1_uif *vsp1_uif_create(struct vsp1_device *vsp1, unsigned int index)
 	uif->entity.index = index;
 	uif->entity.codes = uif_codes;
 	uif->entity.num_codes = ARRAY_SIZE(uif_codes);
+	uif->entity.min_width = UIF_MIN_SIZE;
+	uif->entity.min_height = UIF_MIN_SIZE;
+	uif->entity.max_width = UIF_MAX_SIZE;
+	uif->entity.max_height = UIF_MAX_SIZE;
 
 	/* The datasheet names the two UIF instances UIF4 and UIF5. */
 	sprintf(name, "uif.%u", index + 4);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index 30662cfdf837..cd6c5592221b 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -531,7 +531,7 @@ static unsigned int wpf_max_width(struct vsp1_entity *entity,
 {
 	struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
 
-	return wpf->flip.rotate ? 256 : wpf->max_width;
+	return wpf->flip.rotate ? 256 : wpf->entity.max_width;
 }
 
 static void wpf_partition(struct vsp1_entity *entity,
@@ -567,12 +567,15 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index)
 	if (wpf == NULL)
 		return ERR_PTR(-ENOMEM);
 
+	wpf->entity.min_width = RWPF_MIN_WIDTH;
+	wpf->entity.min_height = RWPF_MIN_HEIGHT;
+
 	if (vsp1->info->gen == 2) {
-		wpf->max_width = WPF_GEN2_MAX_WIDTH;
-		wpf->max_height = WPF_GEN2_MAX_HEIGHT;
+		wpf->entity.max_width = WPF_GEN2_MAX_WIDTH;
+		wpf->entity.max_height = WPF_GEN2_MAX_HEIGHT;
 	} else {
-		wpf->max_width = WPF_GEN3_MAX_WIDTH;
-		wpf->max_height = WPF_GEN3_MAX_HEIGHT;
+		wpf->entity.max_width = WPF_GEN3_MAX_WIDTH;
+		wpf->entity.max_height = WPF_GEN3_MAX_HEIGHT;
 	}
 
 	wpf->entity.ops = &wpf_entity_ops;
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/8] media: renesas: vsp1: Fix code checks in frame size enumeration
  2025-07-03 22:38 [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 1/8] media: renesas: vsp1: Store supported media bus codes in vsp1_entity Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 2/8] media: renesas: vsp1: Store size limits " Laurent Pinchart
@ 2025-07-03 22:38 ` Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 4/8] media: renesas: vsp1: Fix crop left and top clamping on RPF Laurent Pinchart
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-07-03 22:38 UTC (permalink / raw)
  To: linux-media
  Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen

The media bus code passed to the .enum_frame_size() operation for the
sink pad is required to be supported by the device, but not to match the
current format. All entities that use the vsp1_subdev_enum_frame_size()
helper, as well as the SRU and UDS entities that implement the operation
manually, perform the check incorrectly.

Fix the issue by implementing the correct code check in the
vsp1_subdev_enum_frame_size(). For the SRU and UDS, to avoid duplicating
code, use the vsp1_subdev_enum_frame_size() as a base and override the
enumerated size on the source pad with entity-specific constraints.

While at it, include the missing <linux/mutex.h> as the code locks
mutexes.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
Changes since v1:

- Include <linux/cleanup.h> and <linux/mutex.h>
---
 .../media/platform/renesas/vsp1/vsp1_entity.c | 49 ++++++++++++-------
 .../media/platform/renesas/vsp1/vsp1_sru.c    | 38 +++++++-------
 .../media/platform/renesas/vsp1/vsp1_uds.c    | 38 +++++++-------
 3 files changed, 64 insertions(+), 61 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 04b7ae6fb935..892a2adfdf3a 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -7,8 +7,10 @@
  * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
  */
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/gfp.h>
+#include <linux/mutex.h>
 
 #include <media/media-entity.h>
 #include <media/v4l2-ctrls.h>
@@ -238,42 +240,51 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
 				struct v4l2_subdev_frame_size_enum *fse)
 {
 	struct vsp1_entity *entity = to_vsp1_entity(subdev);
-	struct v4l2_subdev_state *state;
-	struct v4l2_mbus_framefmt *format;
-	int ret = 0;
 
-	state = vsp1_entity_get_state(entity, sd_state, fse->which);
-	if (!state)
+	if (fse->index)
 		return -EINVAL;
 
-	format = v4l2_subdev_state_get_format(state, fse->pad);
-
-	mutex_lock(&entity->lock);
-
-	if (fse->index || fse->code != format->code) {
-		ret = -EINVAL;
-		goto done;
-	}
-
 	if (fse->pad == 0) {
+		unsigned int i;
+
+		for (i = 0; i < entity->num_codes; ++i) {
+			if (fse->code == entity->codes[i])
+				break;
+		}
+
+		if (i == entity->num_codes)
+			return -EINVAL;
+
 		fse->min_width = entity->min_width;
 		fse->max_width = entity->max_width;
 		fse->min_height = entity->min_height;
 		fse->max_height = entity->max_height;
 	} else {
+		struct v4l2_subdev_state *state;
+		struct v4l2_mbus_framefmt *format;
+
+		state = vsp1_entity_get_state(entity, sd_state, fse->which);
+		if (!state)
+			return -EINVAL;
+
 		/*
-		 * The size on the source pad are fixed and always identical to
-		 * the size on the sink pad.
+		 * The media bus code and size on the source pad are fixed and
+		 * always identical to the sink pad.
 		 */
+		format = v4l2_subdev_state_get_format(state, 0);
+
+		guard(mutex)(&entity->lock);
+
+		if (fse->code != format->code)
+			return -EINVAL;
+
 		fse->min_width = format->width;
 		fse->max_width = format->width;
 		fse->min_height = format->height;
 		fse->max_height = format->height;
 	}
 
-done:
-	mutex_unlock(&entity->lock);
-	return ret;
+	return 0;
 }
 
 /*
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
index 1dc34e6a510d..37fd36d09045 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -7,8 +7,10 @@
  * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
  */
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/gfp.h>
+#include <linux/mutex.h>
 
 #include <media/v4l2-subdev.h>
 
@@ -116,29 +118,25 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_frame_size_enum *fse)
 {
 	struct vsp1_sru *sru = to_sru(subdev);
-	struct v4l2_subdev_state *state;
-	struct v4l2_mbus_framefmt *format;
-	int ret = 0;
+	int ret;
 
-	state = vsp1_entity_get_state(&sru->entity, sd_state, fse->which);
-	if (!state)
-		return -EINVAL;
+	ret = vsp1_subdev_enum_frame_size(subdev, sd_state, fse);
+	if (ret)
+		return ret;
 
-	format = v4l2_subdev_state_get_format(state, SRU_PAD_SINK);
+	if (fse->pad == SRU_PAD_SOURCE) {
+		struct v4l2_subdev_state *state;
+		struct v4l2_mbus_framefmt *format;
 
-	mutex_lock(&sru->entity.lock);
+		state = vsp1_entity_get_state(&sru->entity, sd_state,
+					      fse->which);
+		if (!state)
+			return -EINVAL;
 
-	if (fse->index || fse->code != format->code) {
-		ret = -EINVAL;
-		goto done;
-	}
+		format = v4l2_subdev_state_get_format(state, SRU_PAD_SINK);
+
+		guard(mutex)(&sru->entity.lock);
 
-	if (fse->pad == SRU_PAD_SINK) {
-		fse->min_width = SRU_MIN_SIZE;
-		fse->max_width = SRU_MAX_SIZE;
-		fse->min_height = SRU_MIN_SIZE;
-		fse->max_height = SRU_MAX_SIZE;
-	} else {
 		fse->min_width = format->width;
 		fse->min_height = format->height;
 		if (format->width <= SRU_MAX_SIZE / 2 &&
@@ -151,9 +149,7 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
 		}
 	}
 
-done:
-	mutex_unlock(&sru->entity.lock);
-	return ret;
+	return 0;
 }
 
 static void sru_try_format(struct vsp1_sru *sru,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index 8006d49ffbea..dd4722315c56 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -7,8 +7,10 @@
  * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
  */
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/gfp.h>
+#include <linux/mutex.h>
 
 #include <media/v4l2-subdev.h>
 
@@ -121,38 +123,32 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
 			       struct v4l2_subdev_frame_size_enum *fse)
 {
 	struct vsp1_uds *uds = to_uds(subdev);
-	struct v4l2_subdev_state *state;
-	struct v4l2_mbus_framefmt *format;
-	int ret = 0;
+	int ret;
 
-	state = vsp1_entity_get_state(&uds->entity, sd_state, fse->which);
-	if (!state)
-		return -EINVAL;
+	ret = vsp1_subdev_enum_frame_size(subdev, sd_state, fse);
+	if (ret)
+		return ret;
 
-	format = v4l2_subdev_state_get_format(state, UDS_PAD_SINK);
+	if (fse->pad == UDS_PAD_SOURCE) {
+		struct v4l2_subdev_state *state;
+		struct v4l2_mbus_framefmt *format;
 
-	mutex_lock(&uds->entity.lock);
+		state = vsp1_entity_get_state(&uds->entity, sd_state,
+					      fse->which);
+		if (!state)
+			return -EINVAL;
 
-	if (fse->index || fse->code != format->code) {
-		ret = -EINVAL;
-		goto done;
-	}
+		format = v4l2_subdev_state_get_format(state, UDS_PAD_SINK);
+
+		guard(mutex)(&uds->entity.lock);
 
-	if (fse->pad == UDS_PAD_SINK) {
-		fse->min_width = UDS_MIN_SIZE;
-		fse->max_width = UDS_MAX_SIZE;
-		fse->min_height = UDS_MIN_SIZE;
-		fse->max_height = UDS_MAX_SIZE;
-	} else {
 		uds_output_limits(format->width, &fse->min_width,
 				  &fse->max_width);
 		uds_output_limits(format->height, &fse->min_height,
 				  &fse->max_height);
 	}
 
-done:
-	mutex_unlock(&uds->entity.lock);
-	return ret;
+	return 0;
 }
 
 static void uds_try_format(struct vsp1_uds *uds,
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/8] media: renesas: vsp1: Fix crop left and top clamping on RPF
  2025-07-03 22:38 [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
                   ` (2 preceding siblings ...)
  2025-07-03 22:38 ` [PATCH v2 3/8] media: renesas: vsp1: Fix code checks in frame size enumeration Laurent Pinchart
@ 2025-07-03 22:38 ` Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 5/8] media: renesas: vsp1: Fix crop width and height " Laurent Pinchart
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-07-03 22:38 UTC (permalink / raw)
  To: linux-media
  Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen

The RPF doesn't enforces the alignment constraint on the sink pad
format, which could have an odd size, possibly down to 1x1. In that
case, the upper bounds for the left and top coordinates clamping would
become negative, cast to a very large positive value. Incorrect crop
rectangle coordinates would then be incorrectly accepted.

A second issue can occur when the requested left and top coordinates are
negative. They are cast to a large unsigned value, clamped to the
maximum. While the calculation will produce valid values for the
hardware, this is not compliant with the V4L2 specification that
requires values to be adjusted to the closest valid value.

Fix both issues by switching to signed clamping, with an explicit
minimum to adjust negative values, and adjusting the clamp bounds to
avoid negative upper bounds.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../media/platform/renesas/vsp1/vsp1_rwpf.c   | 28 ++++++++++++++++---
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index 56464875a6c5..ffc1b3ab54e2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -201,6 +201,8 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
 				   struct v4l2_subdev_state *sd_state,
 				   struct v4l2_subdev_selection *sel)
 {
+	unsigned int min_width = RWPF_MIN_WIDTH;
+	unsigned int min_height = RWPF_MIN_HEIGHT;
 	struct vsp1_rwpf *rwpf = to_rwpf(subdev);
 	struct v4l2_subdev_state *state;
 	struct v4l2_mbus_framefmt *format;
@@ -229,18 +231,36 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
 	format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
 
 	/*
-	 * Restrict the crop rectangle coordinates to multiples of 2 to avoid
-	 * shifting the color plane.
+	 * For YUV formats, restrict the crop rectangle coordinates to multiples
+	 * of 2 to avoid shifting the color plane.
 	 */
 	if (format->code == MEDIA_BUS_FMT_AYUV8_1X32) {
 		sel->r.left = ALIGN(sel->r.left, 2);
 		sel->r.top = ALIGN(sel->r.top, 2);
 		sel->r.width = round_down(sel->r.width, 2);
 		sel->r.height = round_down(sel->r.height, 2);
+
+		/*
+		 * The RPF doesn't enforces the alignment constraint on the sink
+		 * pad format, which could have an odd size, possibly down to
+		 * 1x1. In that case, the minimum width and height would be
+		 * smaller than the sink pad format, leading to a negative upper
+		 * bound in the left and top clamping. Clamp the minimum width
+		 * and height to the format width and height to avoid this.
+		 *
+		 * In such a situation, odd values for the crop rectangle size
+		 * would be accepted when clamping the width and height below.
+		 * While that would create an invalid hardware configuration,
+		 * the video device enforces proper alignment of the pixel
+		 * format, and the mismatch will then result in link validation
+		 * failure. Incorrect operation of the hardware is not possible.
+		 */
+		min_width = min(ALIGN(min_width, 2), format->width);
+		min_height = min(ALIGN(min_height, 2), format->height);
 	}
 
-	sel->r.left = min_t(unsigned int, sel->r.left, format->width - 2);
-	sel->r.top = min_t(unsigned int, sel->r.top, format->height - 2);
+	sel->r.left = clamp_t(int, sel->r.left, 0, format->width - min_width);
+	sel->r.top = clamp_t(int, sel->r.top, 0, format->height - min_height);
 	sel->r.width = min_t(unsigned int, sel->r.width,
 			     format->width - sel->r.left);
 	sel->r.height = min_t(unsigned int, sel->r.height,
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/8] media: renesas: vsp1: Fix crop width and height clamping on RPF
  2025-07-03 22:38 [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
                   ` (3 preceding siblings ...)
  2025-07-03 22:38 ` [PATCH v2 4/8] media: renesas: vsp1: Fix crop left and top clamping on RPF Laurent Pinchart
@ 2025-07-03 22:38 ` Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 6/8] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration Laurent Pinchart
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-07-03 22:38 UTC (permalink / raw)
  To: linux-media
  Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen

The vsp1 driver doesn't enforce a minimum value on the RPF crop rectangle
width and height. Empty rectangles are accepted, leading to incorrect
hardware behaviour. Fix it by adding minimum width and height
constraints to the value clamping.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/vsp1/vsp1_rwpf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index ffc1b3ab54e2..ccc7608f538d 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -261,9 +261,9 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
 
 	sel->r.left = clamp_t(int, sel->r.left, 0, format->width - min_width);
 	sel->r.top = clamp_t(int, sel->r.top, 0, format->height - min_height);
-	sel->r.width = min_t(unsigned int, sel->r.width,
+	sel->r.width = clamp(sel->r.width, min_width,
 			     format->width - sel->r.left);
-	sel->r.height = min_t(unsigned int, sel->r.height,
+	sel->r.height = clamp(sel->r.height, min_height,
 			      format->height - sel->r.top);
 
 	crop = v4l2_subdev_state_get_crop(state, RWPF_PAD_SINK);
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 6/8] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration
  2025-07-03 22:38 [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
                   ` (4 preceding siblings ...)
  2025-07-03 22:38 ` [PATCH v2 5/8] media: renesas: vsp1: Fix crop width and height " Laurent Pinchart
@ 2025-07-03 22:38 ` Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 7/8] media: renesas: vsp1: Fix format propagation on the BRX Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 8/8] media: renesas: vsp1: Implement control events Laurent Pinchart
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-07-03 22:38 UTC (permalink / raw)
  To: linux-media
  Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen

The RWPF can't freely convert between all input and output formats. They
support RGB <-> YUV conversion, but HSV formats can't be converted. Fix
the media bus code and frame size enumeration to take this into account
on the source pad.

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

- Replace hardcoded pad number with RWPF_PAD_SINK
---
 .../media/platform/renesas/vsp1/vsp1_rwpf.c   | 80 +++++++++++++++++--
 1 file changed, 74 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index ccc7608f538d..c9e6fb98f863 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -18,29 +18,97 @@
  * V4L2 Subdevice Operations
  */
 
+/* Keep HSV last. */
 static const unsigned int rwpf_codes[] = {
+	MEDIA_BUS_FMT_AYUV8_1X32,
 	MEDIA_BUS_FMT_ARGB8888_1X32,
 	MEDIA_BUS_FMT_AHSV8888_1X32,
-	MEDIA_BUS_FMT_AYUV8_1X32,
 };
 
 static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev,
 				    struct v4l2_subdev_state *sd_state,
 				    struct v4l2_subdev_mbus_code_enum *code)
 {
-	if (code->index >= ARRAY_SIZE(rwpf_codes))
+	struct vsp1_entity *entity = to_vsp1_entity(subdev);
+	struct v4l2_subdev_state *state;
+	struct v4l2_mbus_framefmt *format;
+
+	if (code->pad == RWPF_PAD_SINK)
+		return vsp1_subdev_enum_mbus_code(subdev, sd_state, code);
+
+	state = vsp1_entity_get_state(entity, sd_state, code->which);
+	if (!state)
 		return -EINVAL;
 
-	code->code = rwpf_codes[code->index];
+	format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
 
-	if (code->pad == RWPF_PAD_SOURCE &&
-	    code->code == MEDIA_BUS_FMT_AYUV8_1X32)
+	guard(mutex)(&entity->lock);
+
+	/*
+	 * The RWPF supports conversion between RGB and YUV formats, but HSV
+	 * formats can't be converted.
+	 */
+	if (format->code == MEDIA_BUS_FMT_AHSV8888_1X32) {
+		if (code->index)
+			return -EINVAL;
+
+		code->code = MEDIA_BUS_FMT_AHSV8888_1X32;
+	} else {
+		if (code->index >= ARRAY_SIZE(rwpf_codes) - 1)
+			return -EINVAL;
+
+		code->code = rwpf_codes[code->index];
+	}
+
+	if (code->code == MEDIA_BUS_FMT_AYUV8_1X32)
 		code->flags = V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC
 			    | V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION;
 
 	return 0;
 }
 
+static int vsp1_rwpf_enum_frame_size(struct v4l2_subdev *subdev,
+				     struct v4l2_subdev_state *sd_state,
+				     struct v4l2_subdev_frame_size_enum *fse)
+{
+	struct vsp1_entity *entity = to_vsp1_entity(subdev);
+	struct v4l2_subdev_state *state;
+	struct v4l2_mbus_framefmt *format;
+
+	if (fse->pad == RWPF_PAD_SINK)
+		return vsp1_subdev_enum_frame_size(subdev, sd_state, fse);
+
+	if (fse->index)
+		return -EINVAL;
+
+	state = vsp1_entity_get_state(entity, sd_state, fse->which);
+	if (!state)
+		return -EINVAL;
+
+	format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
+
+	guard(mutex)(&entity->lock);
+
+	/*
+	 * The RWPF supports conversion between RGB and YUV formats, but
+	 * HSV formats can't be converted.
+	 */
+	if ((format->code == MEDIA_BUS_FMT_AHSV8888_1X32) !=
+	    (fse->code == MEDIA_BUS_FMT_AHSV8888_1X32))
+		return -EINVAL;
+
+	/*
+	 * The size on the source pad is fixed and always identical to
+	 * the sink pad.
+	 */
+	fse->min_width = format->width;
+	fse->max_width = format->width;
+	fse->min_height = format->height;
+	fse->max_height = format->height;
+
+	return 0;
+}
+
 static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
 				struct v4l2_subdev_state *sd_state,
 				struct v4l2_subdev_format *fmt)
@@ -281,7 +349,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
 
 static const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = {
 	.enum_mbus_code = vsp1_rwpf_enum_mbus_code,
-	.enum_frame_size = vsp1_subdev_enum_frame_size,
+	.enum_frame_size = vsp1_rwpf_enum_frame_size,
 	.get_fmt = vsp1_subdev_get_pad_format,
 	.set_fmt = vsp1_rwpf_set_format,
 	.get_selection = vsp1_rwpf_get_selection,
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 7/8] media: renesas: vsp1: Fix format propagation on the BRX
  2025-07-03 22:38 [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
                   ` (5 preceding siblings ...)
  2025-07-03 22:38 ` [PATCH v2 6/8] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration Laurent Pinchart
@ 2025-07-03 22:38 ` Laurent Pinchart
  2025-07-03 22:38 ` [PATCH v2 8/8] media: renesas: vsp1: Implement control events Laurent Pinchart
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-07-03 22:38 UTC (permalink / raw)
  To: linux-media
  Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen

The format width and height is never propagated to the BRX source pad,
leaving its initial configuration invalid. Propagate the whole format
from the first sink pad to the source pad instead of only propagating
the media bus code. This fixes compliance with the subdev format
propagation rules.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/vsp1/vsp1_brx.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index dd651cef93e4..911359faa600 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -156,14 +156,20 @@ static int brx_set_format(struct v4l2_subdev *subdev,
 		compose->height = format->height;
 	}
 
-	/* Propagate the format code to all pads. */
+	/*
+	 * Propagate the format code to all pads, and the whole format to the
+	 * source pad.
+	 */
 	if (fmt->pad == BRX_PAD_SINK(0)) {
 		unsigned int i;
 
-		for (i = 0; i <= brx->entity.source_pad; ++i) {
+		for (i = 0; i < brx->entity.source_pad; ++i) {
 			format = v4l2_subdev_state_get_format(state, i);
 			format->code = fmt->format.code;
 		}
+
+		format = v4l2_subdev_state_get_format(state, i);
+		*format = fmt->format;
 	}
 
 done:
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 8/8] media: renesas: vsp1: Implement control events
  2025-07-03 22:38 [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
                   ` (6 preceding siblings ...)
  2025-07-03 22:38 ` [PATCH v2 7/8] media: renesas: vsp1: Fix format propagation on the BRX Laurent Pinchart
@ 2025-07-03 22:38 ` Laurent Pinchart
  7 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-07-03 22:38 UTC (permalink / raw)
  To: linux-media
  Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen

The V4L2 API requires drivers that expose controls to implement control
notification events. This is enforced by v4l2-compliance. Add event
handling to the VSP1 entities that create controls to fix the compliance
failures.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/vsp1/vsp1_brx.c    | 1 +
 drivers/media/platform/renesas/vsp1/vsp1_clu.c    | 1 +
 drivers/media/platform/renesas/vsp1/vsp1_entity.c | 9 +++++++++
 drivers/media/platform/renesas/vsp1/vsp1_entity.h | 2 ++
 drivers/media/platform/renesas/vsp1/vsp1_histo.c  | 1 +
 drivers/media/platform/renesas/vsp1/vsp1_lut.c    | 1 +
 drivers/media/platform/renesas/vsp1/vsp1_rwpf.c   | 1 +
 drivers/media/platform/renesas/vsp1/vsp1_sru.c    | 1 +
 8 files changed, 17 insertions(+)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index 911359faa600..b1a2c68e9944 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -269,6 +269,7 @@ static const struct v4l2_subdev_pad_ops brx_pad_ops = {
 };
 
 static const struct v4l2_subdev_ops brx_ops = {
+	.core	= &vsp1_entity_core_ops,
 	.pad    = &brx_pad_ops,
 };
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_clu.c b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
index a56c038a2c71..04c466c4da81 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
@@ -130,6 +130,7 @@ static const struct v4l2_subdev_pad_ops clu_pad_ops = {
 };
 
 static const struct v4l2_subdev_ops clu_ops = {
+	.core	= &vsp1_entity_core_ops,
 	.pad    = &clu_pad_ops,
 };
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 892a2adfdf3a..839b75b62ceb 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -14,6 +14,7 @@
 
 #include <media/media-entity.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-subdev.h>
 
 #include "vsp1.h"
@@ -396,6 +397,11 @@ static const struct v4l2_subdev_internal_ops vsp1_entity_internal_ops = {
 	.init_state = vsp1_entity_init_state,
 };
 
+const struct v4l2_subdev_core_ops vsp1_entity_core_ops = {
+	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
+};
+
 /* -----------------------------------------------------------------------------
  * Media Operations
  */
@@ -639,6 +645,9 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,
 	subdev->entity.ops = &vsp1->media_ops;
 	subdev->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
+	if (ops->core == &vsp1_entity_core_ops)
+		subdev->flags |= V4L2_SUBDEV_FL_HAS_EVENTS;
+
 	snprintf(subdev->name, sizeof(subdev->name), "%s %s",
 		 dev_name(vsp1->dev), name);
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index 88daf83cd1ab..c0c1fe7d3e40 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -143,6 +143,8 @@ static inline struct vsp1_entity *to_vsp1_entity(struct v4l2_subdev *subdev)
 	return container_of(subdev, struct vsp1_entity, subdev);
 }
 
+extern const struct v4l2_subdev_core_ops vsp1_entity_core_ops;
+
 int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,
 		     const char *name, unsigned int num_pads,
 		     const struct v4l2_subdev_ops *ops, u32 function);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_histo.c b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
index a1b3671a0873..158d01aa5e81 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
@@ -370,6 +370,7 @@ static const struct v4l2_subdev_pad_ops histo_pad_ops = {
 };
 
 static const struct v4l2_subdev_ops histo_ops = {
+	.core	= &vsp1_entity_core_ops,
 	.pad    = &histo_pad_ops,
 };
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lut.c b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
index 2ec4d596465d..94bdedcc5c92 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
@@ -106,6 +106,7 @@ static const struct v4l2_subdev_pad_ops lut_pad_ops = {
 };
 
 static const struct v4l2_subdev_ops lut_ops = {
+	.core	= &vsp1_entity_core_ops,
 	.pad    = &lut_pad_ops,
 };
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index c9e6fb98f863..c72518b29f84 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -357,6 +357,7 @@ static const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = {
 };
 
 const struct v4l2_subdev_ops vsp1_rwpf_subdev_ops = {
+	.core	= &vsp1_entity_core_ops,
 	.pad    = &vsp1_rwpf_pad_ops,
 };
 
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
index 37fd36d09045..94149da0c900 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -252,6 +252,7 @@ static const struct v4l2_subdev_pad_ops sru_pad_ops = {
 };
 
 static const struct v4l2_subdev_ops sru_ops = {
+	.core	= &vsp1_entity_core_ops,
 	.pad    = &sru_pad_ops,
 };
 
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-07-03 22:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-03 22:38 [PATCH v2 0/8] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
2025-07-03 22:38 ` [PATCH v2 1/8] media: renesas: vsp1: Store supported media bus codes in vsp1_entity Laurent Pinchart
2025-07-03 22:38 ` [PATCH v2 2/8] media: renesas: vsp1: Store size limits " Laurent Pinchart
2025-07-03 22:38 ` [PATCH v2 3/8] media: renesas: vsp1: Fix code checks in frame size enumeration Laurent Pinchart
2025-07-03 22:38 ` [PATCH v2 4/8] media: renesas: vsp1: Fix crop left and top clamping on RPF Laurent Pinchart
2025-07-03 22:38 ` [PATCH v2 5/8] media: renesas: vsp1: Fix crop width and height " Laurent Pinchart
2025-07-03 22:38 ` [PATCH v2 6/8] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration Laurent Pinchart
2025-07-03 22:38 ` [PATCH v2 7/8] media: renesas: vsp1: Fix format propagation on the BRX Laurent Pinchart
2025-07-03 22:38 ` [PATCH v2 8/8] media: renesas: vsp1: Implement control events Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).