* [PATCH 1/7] media: renesas: vsp1: Store supported media bus codes in vsp1_entity
2025-04-29 23:53 [PATCH 0/7] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
@ 2025-04-29 23:53 ` Laurent Pinchart
2025-05-28 14:32 ` Jacopo Mondi
2025-04-29 23:53 ` [PATCH 2/7] media: renesas: vsp1: Store size limits " Laurent Pinchart
` (5 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2025-04-29 23:53 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>
---
.../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_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 +++++-----------
13 files changed, 86 insertions(+), 141 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 9f93ae86b1da..e658a6ee5793 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -176,8 +176,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
@@ -186,16 +184,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;
@@ -285,15 +282,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.
@@ -301,7 +296,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)
{
@@ -332,12 +326,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 ce4a09610164..6b42fd5ad7e8 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -112,6 +112,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;
@@ -180,13 +183,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_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] 19+ messages in thread* Re: [PATCH 1/7] media: renesas: vsp1: Store supported media bus codes in vsp1_entity
2025-04-29 23:53 ` [PATCH 1/7] media: renesas: vsp1: Store supported media bus codes in vsp1_entity Laurent Pinchart
@ 2025-05-28 14:32 ` Jacopo Mondi
0 siblings, 0 replies; 19+ messages in thread
From: Jacopo Mondi @ 2025-05-28 14:32 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Jacopo Mondi,
Tomi Valkeinen
Hi Laurent
On Wed, Apr 30, 2025 at 02:53:16AM +0300, Laurent Pinchart wrote:
> 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@ideasonboard.com>
Thanks
j
> ---
> .../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_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 +++++-----------
> 13 files changed, 86 insertions(+), 141 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 9f93ae86b1da..e658a6ee5793 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> @@ -176,8 +176,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
> @@ -186,16 +184,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;
> @@ -285,15 +282,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.
> @@ -301,7 +296,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)
> {
> @@ -332,12 +326,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 ce4a09610164..6b42fd5ad7e8 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
> @@ -112,6 +112,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;
> @@ -180,13 +183,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_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 [flat|nested] 19+ messages in thread
* [PATCH 2/7] media: renesas: vsp1: Store size limits in vsp1_entity
2025-04-29 23:53 [PATCH 0/7] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
2025-04-29 23:53 ` [PATCH 1/7] media: renesas: vsp1: Store supported media bus codes in vsp1_entity Laurent Pinchart
@ 2025-04-29 23:53 ` Laurent Pinchart
2025-05-28 14:43 ` Jacopo Mondi
2025-04-29 23:53 ` [PATCH 3/7] media: renesas: vsp1: Fix code checks in frame size enumeration Laurent Pinchart
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2025-04-29 23:53 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>
---
.../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_lif.c | 26 ++++-----------
.../media/platform/renesas/vsp1/vsp1_lut.c | 32 ++++---------------
.../media/platform/renesas/vsp1/vsp1_rpf.c | 5 ++-
.../media/platform/renesas/vsp1/vsp1_rwpf.c | 19 +++--------
.../media/platform/renesas/vsp1/vsp1_rwpf.h | 3 --
.../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 | 10 +++---
15 files changed, 76 insertions(+), 159 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 e658a6ee5793..0d4fe0028b13 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -222,10 +222,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
@@ -234,9 +230,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;
@@ -257,10 +251,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
@@ -282,22 +276,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;
@@ -334,9 +321,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 6b42fd5ad7e8..5542f6446b16 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -114,6 +114,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;
@@ -182,16 +186,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_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 9f2744af54bc..f23a0f770474 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -418,12 +418,11 @@ 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.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..83ff2c266038 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -44,17 +44,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 +114,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 +264,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,
@@ -312,6 +301,8 @@ 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);
+ rwpf->entity.min_width = RWPF_MIN_WIDTH;
+ rwpf->entity.min_height = RWPF_MIN_HEIGHT;
v4l2_ctrl_handler_init(&rwpf->ctrls, ncontrols + 1);
v4l2_ctrl_new_std(&rwpf->ctrls, &vsp1_rwpf_ctrl_ops,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
index 5ac9f0a6fafc..1a65ff141501 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
@@ -36,9 +36,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 49af637c8186..349acdae83c7 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -525,7 +525,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,
@@ -562,11 +562,11 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index)
return ERR_PTR(-ENOMEM);
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] 19+ messages in thread* Re: [PATCH 2/7] media: renesas: vsp1: Store size limits in vsp1_entity
2025-04-29 23:53 ` [PATCH 2/7] media: renesas: vsp1: Store size limits " Laurent Pinchart
@ 2025-05-28 14:43 ` Jacopo Mondi
2025-06-25 22:44 ` Laurent Pinchart
0 siblings, 1 reply; 19+ messages in thread
From: Jacopo Mondi @ 2025-05-28 14:43 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Jacopo Mondi,
Tomi Valkeinen
Hi Laurent
On Wed, Apr 30, 2025 at 02:53:17AM +0300, Laurent Pinchart wrote:
> 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>
> ---
> .../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_lif.c | 26 ++++-----------
> .../media/platform/renesas/vsp1/vsp1_lut.c | 32 ++++---------------
> .../media/platform/renesas/vsp1/vsp1_rpf.c | 5 ++-
> .../media/platform/renesas/vsp1/vsp1_rwpf.c | 19 +++--------
> .../media/platform/renesas/vsp1/vsp1_rwpf.h | 3 --
> .../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 | 10 +++---
> 15 files changed, 76 insertions(+), 159 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 e658a6ee5793..0d4fe0028b13 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> @@ -222,10 +222,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
> @@ -234,9 +230,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;
> @@ -257,10 +251,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
> @@ -282,22 +276,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;
> @@ -334,9 +321,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 6b42fd5ad7e8..5542f6446b16 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
> @@ -114,6 +114,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;
>
> @@ -182,16 +186,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_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 9f2744af54bc..f23a0f770474 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> @@ -418,12 +418,11 @@ 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.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..83ff2c266038 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> @@ -44,17 +44,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 +114,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 +264,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,
> @@ -312,6 +301,8 @@ 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);
> + rwpf->entity.min_width = RWPF_MIN_WIDTH;
> + rwpf->entity.min_height = RWPF_MIN_HEIGHT;
I wonder if it wouldn't be clearer to initialize both min and max in
the wpf an rpf modules, took me a bit to grasp where max sizes where
set
>
> v4l2_ctrl_handler_init(&rwpf->ctrls, ncontrols + 1);
> v4l2_ctrl_new_std(&rwpf->ctrls, &vsp1_rwpf_ctrl_ops,
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
> index 5ac9f0a6fafc..1a65ff141501 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
> @@ -36,9 +36,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 49af637c8186..349acdae83c7 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> @@ -525,7 +525,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,
> @@ -562,11 +562,11 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index)
> return ERR_PTR(-ENOMEM);
>
> 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;
Minor thing though
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Thanks
j
> }
>
> wpf->entity.ops = &wpf_entity_ops;
> --
> Regards,
>
> Laurent Pinchart
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 2/7] media: renesas: vsp1: Store size limits in vsp1_entity
2025-05-28 14:43 ` Jacopo Mondi
@ 2025-06-25 22:44 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2025-06-25 22:44 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Tomi Valkeinen
Hi Jacopo,
On Wed, May 28, 2025 at 04:43:28PM +0200, Jacopo Mondi wrote:
> On Wed, Apr 30, 2025 at 02:53:17AM +0300, Laurent Pinchart wrote:
> > 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>
> > ---
> > .../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_lif.c | 26 ++++-----------
> > .../media/platform/renesas/vsp1/vsp1_lut.c | 32 ++++---------------
> > .../media/platform/renesas/vsp1/vsp1_rpf.c | 5 ++-
> > .../media/platform/renesas/vsp1/vsp1_rwpf.c | 19 +++--------
> > .../media/platform/renesas/vsp1/vsp1_rwpf.h | 3 --
> > .../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 | 10 +++---
> > 15 files changed, 76 insertions(+), 159 deletions(-)
[snip]
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> > index 304a2f618777..83ff2c266038 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> > @@ -44,17 +44,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 +114,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 +264,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,
> > @@ -312,6 +301,8 @@ 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);
> > + rwpf->entity.min_width = RWPF_MIN_WIDTH;
> > + rwpf->entity.min_height = RWPF_MIN_HEIGHT;
>
> I wonder if it wouldn't be clearer to initialize both min and max in
> the wpf an rpf modules, took me a bit to grasp where max sizes where
> set
I considered the same. I'll change that.
> >
> > v4l2_ctrl_handler_init(&rwpf->ctrls, ncontrols + 1);
> > v4l2_ctrl_new_std(&rwpf->ctrls, &vsp1_rwpf_ctrl_ops,
[snip]
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > index 49af637c8186..349acdae83c7 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > @@ -525,7 +525,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,
> > @@ -562,11 +562,11 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index)
> > return ERR_PTR(-ENOMEM);
> >
> > 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;
>
> Minor thing though
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
I plan to keep your R-b tag with the above change.
> > }
> >
> > wpf->entity.ops = &wpf_entity_ops;
> >
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/7] media: renesas: vsp1: Fix code checks in frame size enumeration
2025-04-29 23:53 [PATCH 0/7] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
2025-04-29 23:53 ` [PATCH 1/7] media: renesas: vsp1: Store supported media bus codes in vsp1_entity Laurent Pinchart
2025-04-29 23:53 ` [PATCH 2/7] media: renesas: vsp1: Store size limits " Laurent Pinchart
@ 2025-04-29 23:53 ` Laurent Pinchart
2025-05-28 14:45 ` Jacopo Mondi
2025-04-29 23:53 ` [PATCH 4/7] media: renesas: vsp1: Fix RPF sink alignment for YUV formats Laurent Pinchart
` (3 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2025-04-29 23:53 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.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_entity.c | 47 +++++++++++--------
.../media/platform/renesas/vsp1/vsp1_sru.c | 36 ++++++--------
.../media/platform/renesas/vsp1/vsp1_uds.c | 36 ++++++--------
3 files changed, 58 insertions(+), 61 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 0d4fe0028b13..a3d4bf2887ec 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -233,42 +233,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..e821eac1cbc2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -116,29 +116,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 +147,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..95c9939ae077 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -121,38 +121,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] 19+ messages in thread* Re: [PATCH 3/7] media: renesas: vsp1: Fix code checks in frame size enumeration
2025-04-29 23:53 ` [PATCH 3/7] media: renesas: vsp1: Fix code checks in frame size enumeration Laurent Pinchart
@ 2025-05-28 14:45 ` Jacopo Mondi
2025-06-25 22:52 ` Laurent Pinchart
0 siblings, 1 reply; 19+ messages in thread
From: Jacopo Mondi @ 2025-05-28 14:45 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Jacopo Mondi,
Tomi Valkeinen
Hi Laurent
On Wed, Apr 30, 2025 at 02:53:18AM +0300, Laurent Pinchart wrote:
> 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.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> .../media/platform/renesas/vsp1/vsp1_entity.c | 47 +++++++++++--------
> .../media/platform/renesas/vsp1/vsp1_sru.c | 36 ++++++--------
> .../media/platform/renesas/vsp1/vsp1_uds.c | 36 ++++++--------
> 3 files changed, 58 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> index 0d4fe0028b13..a3d4bf2887ec 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> @@ -233,42 +233,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);
Should you now include cleanup.h ?
> +
> + 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..e821eac1cbc2 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
> @@ -116,29 +116,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 +147,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..95c9939ae077 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> @@ -121,38 +121,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 [flat|nested] 19+ messages in thread* Re: [PATCH 3/7] media: renesas: vsp1: Fix code checks in frame size enumeration
2025-05-28 14:45 ` Jacopo Mondi
@ 2025-06-25 22:52 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2025-06-25 22:52 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Tomi Valkeinen
On Wed, May 28, 2025 at 04:45:58PM +0200, Jacopo Mondi wrote:
> On Wed, Apr 30, 2025 at 02:53:18AM +0300, Laurent Pinchart wrote:
> > 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.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > .../media/platform/renesas/vsp1/vsp1_entity.c | 47 +++++++++++--------
> > .../media/platform/renesas/vsp1/vsp1_sru.c | 36 ++++++--------
> > .../media/platform/renesas/vsp1/vsp1_uds.c | 36 ++++++--------
> > 3 files changed, 58 insertions(+), 61 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> > index 0d4fe0028b13..a3d4bf2887ec 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> > @@ -233,42 +233,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);
>
> Should you now include cleanup.h ?
I'm in two minds about that. Yes, the guard() macro is provided by
cleanup.h, but the API here is really guard(mutex), which is provided by
mutex.h by using DEFINE_GUARD(), provided by cleanup.h. There's
therefore a guarantee cleanup.h is included.
I think I'll still include cleanup.h in v2, and while at it I'll also
include mutex.h that was missing.
> > +
> > + 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..e821eac1cbc2 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
> > @@ -116,29 +116,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 +147,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..95c9939ae077 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
> > @@ -121,38 +121,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 [flat|nested] 19+ messages in thread
* [PATCH 4/7] media: renesas: vsp1: Fix RPF sink alignment for YUV formats
2025-04-29 23:53 [PATCH 0/7] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
` (2 preceding siblings ...)
2025-04-29 23:53 ` [PATCH 3/7] media: renesas: vsp1: Fix code checks in frame size enumeration Laurent Pinchart
@ 2025-04-29 23:53 ` Laurent Pinchart
2025-05-28 14:49 ` Jacopo Mondi
2025-04-29 23:53 ` [PATCH 5/7] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration Laurent Pinchart
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2025-04-29 23:53 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Kieran Bingham, Jacopo Mondi, Tomi Valkeinen
When reading YUV formats from memory, the hardware requires the crop
rectangle size and position to be aligned to multiples of two, depending
on the horizontal and vertical subsampling factors. The driver doesn't
enforce this, leading to incorrect operation.
As the crop rectangle is implemented on the RPF subdev's sink pad,
enforcing the constraint conditionally based on the subsampling factors
is difficult, as those are only known by the RPF video device. We could
perform the check at pipeline validation time, but that could lead to
confusing -EPIPE errors. As there is very few use cases for odd crop
offsets and sizes with non-subsampled YUV, take the easier and more
user-friendly route of enforcing the constraint on all YUV formats.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_rwpf.c | 41 ++++++++++++-------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index 83ff2c266038..61f7e13ebeee 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -117,6 +117,17 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
RWPF_MIN_WIDTH, rwpf->entity.max_width);
format->height = clamp_t(unsigned int, fmt->format.height,
RWPF_MIN_HEIGHT, rwpf->entity.max_height);
+
+ /*
+ * For YUV formats on the RPF, restrict the size to multiples of 2 to
+ * avoid shifting the color plane.
+ */
+ if (rwpf->entity.type == VSP1_ENTITY_RPF &&
+ format->code == MEDIA_BUS_FMT_AYUV8_1X32) {
+ format->width = ALIGN(format->width, 2);
+ format->height = ALIGN(format->height, 2);
+ }
+
format->field = V4L2_FIELD_NONE;
format->colorspace = fmt->format.colorspace;
@@ -231,23 +242,23 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
/* Make sure the crop rectangle is entirely contained in the image. */
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.
- */
- 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);
- }
-
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.width = min_t(unsigned int, sel->r.width,
- format->width - sel->r.left);
- sel->r.height = min_t(unsigned int, sel->r.height,
- format->height - sel->r.top);
+ sel->r.width = clamp_t(unsigned int, sel->r.width, RWPF_MIN_WIDTH,
+ format->width - sel->r.left);
+ sel->r.height = clamp_t(unsigned int, sel->r.height, RWPF_MIN_HEIGHT,
+ format->height - sel->r.top);
+
+ /*
+ * 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 = round_down(sel->r.left, 2);
+ sel->r.top = round_down(sel->r.top, 2);
+ sel->r.width = ALIGN(sel->r.width, 2);
+ sel->r.height = ALIGN(sel->r.height, 2);
+ }
crop = v4l2_subdev_state_get_crop(state, RWPF_PAD_SINK);
*crop = sel->r;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 4/7] media: renesas: vsp1: Fix RPF sink alignment for YUV formats
2025-04-29 23:53 ` [PATCH 4/7] media: renesas: vsp1: Fix RPF sink alignment for YUV formats Laurent Pinchart
@ 2025-05-28 14:49 ` Jacopo Mondi
2025-06-25 23:38 ` Laurent Pinchart
0 siblings, 1 reply; 19+ messages in thread
From: Jacopo Mondi @ 2025-05-28 14:49 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Jacopo Mondi,
Tomi Valkeinen
Hi Laurent
On Wed, Apr 30, 2025 at 02:53:19AM +0300, Laurent Pinchart wrote:
> When reading YUV formats from memory, the hardware requires the crop
> rectangle size and position to be aligned to multiples of two, depending
> on the horizontal and vertical subsampling factors. The driver doesn't
> enforce this, leading to incorrect operation.
>
> As the crop rectangle is implemented on the RPF subdev's sink pad,
> enforcing the constraint conditionally based on the subsampling factors
> is difficult, as those are only known by the RPF video device. We could
> perform the check at pipeline validation time, but that could lead to
> confusing -EPIPE errors. As there is very few use cases for odd crop
> offsets and sizes with non-subsampled YUV, take the easier and more
> user-friendly route of enforcing the constraint on all YUV formats.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> .../media/platform/renesas/vsp1/vsp1_rwpf.c | 41 ++++++++++++-------
> 1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> index 83ff2c266038..61f7e13ebeee 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> @@ -117,6 +117,17 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> RWPF_MIN_WIDTH, rwpf->entity.max_width);
> format->height = clamp_t(unsigned int, fmt->format.height,
> RWPF_MIN_HEIGHT, rwpf->entity.max_height);
> +
> + /*
> + * For YUV formats on the RPF, restrict the size to multiples of 2 to
> + * avoid shifting the color plane.
> + */
> + if (rwpf->entity.type == VSP1_ENTITY_RPF &&
> + format->code == MEDIA_BUS_FMT_AYUV8_1X32) {
> + format->width = ALIGN(format->width, 2);
> + format->height = ALIGN(format->height, 2);
ALIGN aligns up right ? Is it ok or is it better to read 1 pixel less
than reading memory outside of the region the user asked for ?
> + }
> +
> format->field = V4L2_FIELD_NONE;
>
> format->colorspace = fmt->format.colorspace;
> @@ -231,23 +242,23 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> /* Make sure the crop rectangle is entirely contained in the image. */
> 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.
> - */
> - 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);
> - }
> -
> 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.width = min_t(unsigned int, sel->r.width,
> - format->width - sel->r.left);
> - sel->r.height = min_t(unsigned int, sel->r.height,
> - format->height - sel->r.top);
> + sel->r.width = clamp_t(unsigned int, sel->r.width, RWPF_MIN_WIDTH,
> + format->width - sel->r.left);
> + sel->r.height = clamp_t(unsigned int, sel->r.height, RWPF_MIN_HEIGHT,
> + format->height - sel->r.top);
> +
> + /*
> + * 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 = round_down(sel->r.left, 2);
> + sel->r.top = round_down(sel->r.top, 2);
> + sel->r.width = ALIGN(sel->r.width, 2);
> + sel->r.height = ALIGN(sel->r.height, 2);
The existing code did
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);
is it intentional ?
> + }
>
> crop = v4l2_subdev_state_get_crop(state, RWPF_PAD_SINK);
> *crop = sel->r;
> --
> Regards,
>
> Laurent Pinchart
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 4/7] media: renesas: vsp1: Fix RPF sink alignment for YUV formats
2025-05-28 14:49 ` Jacopo Mondi
@ 2025-06-25 23:38 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2025-06-25 23:38 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Tomi Valkeinen
Hi Jacopo,
On Wed, May 28, 2025 at 04:49:50PM +0200, Jacopo Mondi wrote:
> On Wed, Apr 30, 2025 at 02:53:19AM +0300, Laurent Pinchart wrote:
> > When reading YUV formats from memory, the hardware requires the crop
> > rectangle size and position to be aligned to multiples of two, depending
> > on the horizontal and vertical subsampling factors. The driver doesn't
> > enforce this, leading to incorrect operation.
> >
> > As the crop rectangle is implemented on the RPF subdev's sink pad,
> > enforcing the constraint conditionally based on the subsampling factors
> > is difficult, as those are only known by the RPF video device. We could
> > perform the check at pipeline validation time, but that could lead to
> > confusing -EPIPE errors. As there is very few use cases for odd crop
> > offsets and sizes with non-subsampled YUV, take the easier and more
> > user-friendly route of enforcing the constraint on all YUV formats.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > .../media/platform/renesas/vsp1/vsp1_rwpf.c | 41 ++++++++++++-------
> > 1 file changed, 26 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> > index 83ff2c266038..61f7e13ebeee 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> > @@ -117,6 +117,17 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> > RWPF_MIN_WIDTH, rwpf->entity.max_width);
> > format->height = clamp_t(unsigned int, fmt->format.height,
> > RWPF_MIN_HEIGHT, rwpf->entity.max_height);
> > +
> > + /*
> > + * For YUV formats on the RPF, restrict the size to multiples of 2 to
> > + * avoid shifting the color plane.
> > + */
> > + if (rwpf->entity.type == VSP1_ENTITY_RPF &&
> > + format->code == MEDIA_BUS_FMT_AYUV8_1X32) {
> > + format->width = ALIGN(format->width, 2);
> > + format->height = ALIGN(format->height, 2);
>
> ALIGN aligns up right ? Is it ok or is it better to read 1 pixel less
> than reading memory outside of the region the user asked for ?
This shouldn't actually matter. Link validation will ensure that the
format on the RPF subdev sink pad matches the format on the connected
video device. If this function rounds the value in a way that does not
match the format on the video device, link validation will fail.
I've checked the implementation on the video device, and the driver does
/* Align the width and height for YUV 4:2:2 and 4:2:0 formats. */
width = round_down(width, info->hsub);
height = round_down(height, info->vsub);
/* Clamp the width and height. */
pix->width = clamp(width, info->hsub, VSP1_VIDEO_MAX_WIDTH);
pix->height = clamp(height, info->vsub, VSP1_VIDEO_MAX_HEIGHT);
It seems I can drop the above code, as the video device correctly
handles the hardware requirement for the input size.
> > + }
> > +
> > format->field = V4L2_FIELD_NONE;
> >
> > format->colorspace = fmt->format.colorspace;
> > @@ -231,23 +242,23 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> > /* Make sure the crop rectangle is entirely contained in the image. */
> > 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.
> > - */
> > - 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);
> > - }
> > -
> > 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.width = min_t(unsigned int, sel->r.width,
> > - format->width - sel->r.left);
> > - sel->r.height = min_t(unsigned int, sel->r.height,
> > - format->height - sel->r.top);
> > + sel->r.width = clamp_t(unsigned int, sel->r.width, RWPF_MIN_WIDTH,
> > + format->width - sel->r.left);
> > + sel->r.height = clamp_t(unsigned int, sel->r.height, RWPF_MIN_HEIGHT,
> > + format->height - sel->r.top);
> > +
> > + /*
> > + * 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 = round_down(sel->r.left, 2);
> > + sel->r.top = round_down(sel->r.top, 2);
> > + sel->r.width = ALIGN(sel->r.width, 2);
> > + sel->r.height = ALIGN(sel->r.height, 2);
>
> The existing code did
>
> 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);
>
> is it intentional ?
If width == left + 1 (which the clamping code allows), rounding left up
and width down will cause width to be 0. Going in the other direction
ensures we won't have an empty crop rectangle. But the width can then
exceed the sink format width, if I drop the rounding there. I'll rework
this patch.
> > + }
> >
> > crop = v4l2_subdev_state_get_crop(state, RWPF_PAD_SINK);
> > *crop = sel->r;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/7] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration
2025-04-29 23:53 [PATCH 0/7] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
` (3 preceding siblings ...)
2025-04-29 23:53 ` [PATCH 4/7] media: renesas: vsp1: Fix RPF sink alignment for YUV formats Laurent Pinchart
@ 2025-04-29 23:53 ` Laurent Pinchart
2025-05-28 14:58 ` Jacopo Mondi
2025-04-29 23:53 ` [PATCH 6/7] media: renesas: vsp1: Fix format propagation on the BRX Laurent Pinchart
2025-04-29 23:53 ` [PATCH 7/7] media: renesas: vsp1: Implement control events Laurent Pinchart
6 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2025-04-29 23:53 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>
---
.../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 61f7e13ebeee..bd97fc75eb5b 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -21,29 +21,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, 0);
+
+ 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)
@@ -275,7 +343,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] 19+ messages in thread* Re: [PATCH 5/7] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration
2025-04-29 23:53 ` [PATCH 5/7] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration Laurent Pinchart
@ 2025-05-28 14:58 ` Jacopo Mondi
2025-06-25 23:47 ` Laurent Pinchart
0 siblings, 1 reply; 19+ messages in thread
From: Jacopo Mondi @ 2025-05-28 14:58 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Jacopo Mondi,
Tomi Valkeinen
Hi Laurent
On Wed, Apr 30, 2025 at 02:53:20AM +0300, Laurent Pinchart wrote:
> 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>
> ---
> .../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 61f7e13ebeee..bd97fc75eb5b 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> @@ -21,29 +21,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;
minor nit: could you move this one line up for ... reverse xmas tree
ordering :)
> +
> + 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, 0);
Could you use RWPF_PAD_SINK ?
> +
> + guard(mutex)(&entity->lock);
As a general question, shouldn't we use the state 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;
> +}
All minors
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Thanks
j
> +
> static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_format *fmt)
> @@ -275,7 +343,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 [flat|nested] 19+ messages in thread* Re: [PATCH 5/7] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration
2025-05-28 14:58 ` Jacopo Mondi
@ 2025-06-25 23:47 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2025-06-25 23:47 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Tomi Valkeinen
On Wed, May 28, 2025 at 04:58:45PM +0200, Jacopo Mondi wrote:
> On Wed, Apr 30, 2025 at 02:53:20AM +0300, Laurent Pinchart wrote:
> > 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>
> > ---
> > .../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 61f7e13ebeee..bd97fc75eb5b 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> > @@ -21,29 +21,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;
>
> minor nit: could you move this one line up for ... reverse xmas tree
> ordering :)
state goes first because in my head you get the state and then access
the format from it. As the line are nearly the same length, the breach
of the reverse xmas tree rule didn't shock me. We're really getting into
nitpicking territory :-)
This will go away when switching to the V4L2 subdev active state API. I
have a patch series for that, but need to fix a lockdep issue.
> > +
> > + 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, 0);
>
> Could you use RWPF_PAD_SINK ?
Will do.
> > +
> > + guard(mutex)(&entity->lock);
>
> As a general question, shouldn't we use the state lock ?
The series I mentioned above will handle this. The driver doesn't
currently use the active statue API.
> > +
> > + /*
> > + * 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;
> > +}
>
> All minors
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> > +
> > static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
> > struct v4l2_subdev_state *sd_state,
> > struct v4l2_subdev_format *fmt)
> > @@ -275,7 +343,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 [flat|nested] 19+ messages in thread
* [PATCH 6/7] media: renesas: vsp1: Fix format propagation on the BRX
2025-04-29 23:53 [PATCH 0/7] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
` (4 preceding siblings ...)
2025-04-29 23:53 ` [PATCH 5/7] media: renesas: vsp1: Fix RWPF media bus code and frame size enumeration Laurent Pinchart
@ 2025-04-29 23:53 ` Laurent Pinchart
2025-05-28 15:00 ` Jacopo Mondi
2025-04-29 23:53 ` [PATCH 7/7] media: renesas: vsp1: Implement control events Laurent Pinchart
6 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2025-04-29 23:53 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>
---
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] 19+ messages in thread* Re: [PATCH 6/7] media: renesas: vsp1: Fix format propagation on the BRX
2025-04-29 23:53 ` [PATCH 6/7] media: renesas: vsp1: Fix format propagation on the BRX Laurent Pinchart
@ 2025-05-28 15:00 ` Jacopo Mondi
0 siblings, 0 replies; 19+ messages in thread
From: Jacopo Mondi @ 2025-05-28 15:00 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Jacopo Mondi,
Tomi Valkeinen
Hi Laurent
On Wed, Apr 30, 2025 at 02:53:21AM +0300, Laurent Pinchart wrote:
> 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>
> ---
> 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;
nice catch
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Thanks
j
> }
>
> done:
> --
> Regards,
>
> Laurent Pinchart
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 7/7] media: renesas: vsp1: Implement control events
2025-04-29 23:53 [PATCH 0/7] media: renesas: vsp1: Fix v4l2-compliance failures Laurent Pinchart
` (5 preceding siblings ...)
2025-04-29 23:53 ` [PATCH 6/7] media: renesas: vsp1: Fix format propagation on the BRX Laurent Pinchart
@ 2025-04-29 23:53 ` Laurent Pinchart
2025-05-28 15:06 ` Jacopo Mondi
6 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2025-04-29 23:53 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>
---
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 a3d4bf2887ec..27c172788621 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -12,6 +12,7 @@
#include <media/media-entity.h>
#include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
#include <media/v4l2-subdev.h>
#include "vsp1.h"
@@ -389,6 +390,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
*/
@@ -629,6 +635,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 5542f6446b16..626fb1d5d470 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -142,6 +142,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 bd97fc75eb5b..32b018e21f57 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -351,6 +351,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 e821eac1cbc2..30b482a160c2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -250,6 +250,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] 19+ messages in thread* Re: [PATCH 7/7] media: renesas: vsp1: Implement control events
2025-04-29 23:53 ` [PATCH 7/7] media: renesas: vsp1: Implement control events Laurent Pinchart
@ 2025-05-28 15:06 ` Jacopo Mondi
0 siblings, 0 replies; 19+ messages in thread
From: Jacopo Mondi @ 2025-05-28 15:06 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, linux-renesas-soc, Kieran Bingham, Jacopo Mondi,
Tomi Valkeinen
Hi Laurent
On Wed, Apr 30, 2025 at 02:53:22AM +0300, Laurent Pinchart wrote:
> 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@ideasonboard.com>
Thanks
j
> ---
> 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 a3d4bf2887ec..27c172788621 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
> @@ -12,6 +12,7 @@
>
> #include <media/media-entity.h>
> #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> #include <media/v4l2-subdev.h>
>
> #include "vsp1.h"
> @@ -389,6 +390,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
> */
> @@ -629,6 +635,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 5542f6446b16..626fb1d5d470 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
> @@ -142,6 +142,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 bd97fc75eb5b..32b018e21f57 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
> @@ -351,6 +351,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 e821eac1cbc2..30b482a160c2 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
> @@ -250,6 +250,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 [flat|nested] 19+ messages in thread