* [PATCH v2 01/19] media: renesas: vsp1: Drop vsp1_entity_get_pad_format() wrapper
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 02/19] media: renesas: vsp1: Drop vsp1_entity_get_pad_selection() wrapper Laurent Pinchart
` (18 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham, Laurent Pinchart
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The vsp1_entity_get_pad_format() function is just a wrapper around
v4l2_subdev_state_get_format() without any added value. Drop it and call
v4l2_subdev_state_get_format() directly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
Changes since v1:
- Remove unneeded line breaks
---
.../media/platform/renesas/vsp1/vsp1_brx.c | 18 +++++-----
.../media/platform/renesas/vsp1/vsp1_clu.c | 3 +-
.../media/platform/renesas/vsp1/vsp1_entity.c | 27 +++-----------
.../media/platform/renesas/vsp1/vsp1_entity.h | 4 ---
.../media/platform/renesas/vsp1/vsp1_histo.c | 6 ++--
.../media/platform/renesas/vsp1/vsp1_hsit.c | 5 ++-
.../media/platform/renesas/vsp1/vsp1_lif.c | 4 +--
.../media/platform/renesas/vsp1/vsp1_rpf.c | 10 +++---
.../media/platform/renesas/vsp1/vsp1_rwpf.c | 14 +++-----
.../media/platform/renesas/vsp1/vsp1_sru.c | 31 +++++++---------
.../media/platform/renesas/vsp1/vsp1_uds.c | 35 ++++++++-----------
.../media/platform/renesas/vsp1/vsp1_uif.c | 5 ++-
.../media/platform/renesas/vsp1/vsp1_video.c | 10 +++---
.../media/platform/renesas/vsp1/vsp1_wpf.c | 25 ++++++-------
14 files changed, 73 insertions(+), 124 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index a8535c6e2c46..0eb4d8fe4285 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -119,8 +119,8 @@ static void brx_try_format(struct vsp1_brx *brx,
default:
/* The BRx can't perform format conversion. */
- format = vsp1_entity_get_pad_format(&brx->entity, sd_state,
- BRX_PAD_SINK(0));
+ format = v4l2_subdev_state_get_format(sd_state,
+ BRX_PAD_SINK(0));
fmt->code = format->code;
break;
}
@@ -150,7 +150,7 @@ static int brx_set_format(struct v4l2_subdev *subdev,
brx_try_format(brx, state, fmt->pad, &fmt->format);
- format = vsp1_entity_get_pad_format(&brx->entity, state, fmt->pad);
+ format = v4l2_subdev_state_get_format(state, fmt->pad);
*format = fmt->format;
/* Reset the compose rectangle. */
@@ -169,8 +169,7 @@ static int brx_set_format(struct v4l2_subdev *subdev,
unsigned int i;
for (i = 0; i <= brx->entity.source_pad; ++i) {
- format = vsp1_entity_get_pad_format(&brx->entity,
- state, i);
+ format = v4l2_subdev_state_get_format(state, i);
format->code = fmt->format.code;
}
}
@@ -242,8 +241,7 @@ static int brx_set_selection(struct v4l2_subdev *subdev,
* The compose rectangle top left corner must be inside the output
* frame.
*/
- format = vsp1_entity_get_pad_format(&brx->entity, state,
- brx->entity.source_pad);
+ format = v4l2_subdev_state_get_format(state, brx->entity.source_pad);
sel->r.left = clamp_t(unsigned int, sel->r.left, 0, format->width - 1);
sel->r.top = clamp_t(unsigned int, sel->r.top, 0, format->height - 1);
@@ -251,7 +249,7 @@ static int brx_set_selection(struct v4l2_subdev *subdev,
* Scaling isn't supported, the compose rectangle size must be identical
* to the sink format size.
*/
- format = vsp1_entity_get_pad_format(&brx->entity, state, sel->pad);
+ format = v4l2_subdev_state_get_format(state, sel->pad);
sel->r.width = format->width;
sel->r.height = format->height;
@@ -290,8 +288,8 @@ static void brx_configure_stream(struct vsp1_entity *entity,
unsigned int flags;
unsigned int i;
- format = vsp1_entity_get_pad_format(&brx->entity, brx->entity.state,
- brx->entity.source_pad);
+ format = v4l2_subdev_state_get_format(brx->entity.state,
+ brx->entity.source_pad);
/*
* The hardware is extremely flexible but we have no userspace API to
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_clu.c b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
index 625776a9bda4..1e57676a420c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
@@ -181,8 +181,7 @@ static void clu_configure_stream(struct vsp1_entity *entity,
* The yuv_mode can't be changed during streaming. Cache it internally
* for future runtime configuration calls.
*/
- format = vsp1_entity_get_pad_format(&clu->entity, clu->entity.state,
- CLU_PAD_SINK);
+ format = v4l2_subdev_state_get_format(clu->entity.state, CLU_PAD_SINK);
clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 0a5a7f9cc870..fa748cf89d44 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -127,23 +127,6 @@ vsp1_entity_get_state(struct vsp1_entity *entity,
}
}
-/**
- * vsp1_entity_get_pad_format - Get a pad format from storage for an entity
- * @entity: the entity
- * @sd_state: the state storage
- * @pad: the pad number
- *
- * Return the format stored in the given configuration for an entity's pad. The
- * configuration can be an ACTIVE or TRY configuration.
- */
-struct v4l2_mbus_framefmt *
-vsp1_entity_get_pad_format(struct vsp1_entity *entity,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad)
-{
- return v4l2_subdev_state_get_format(sd_state, pad);
-}
-
/**
* vsp1_entity_get_pad_selection - Get a pad selection from storage for entity
* @entity: the entity
@@ -191,7 +174,7 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
return -EINVAL;
mutex_lock(&entity->lock);
- fmt->format = *vsp1_entity_get_pad_format(entity, state, fmt->pad);
+ fmt->format = *v4l2_subdev_state_get_format(state, fmt->pad);
mutex_unlock(&entity->lock);
return 0;
@@ -238,7 +221,7 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
return -EINVAL;
mutex_lock(&entity->lock);
- format = vsp1_entity_get_pad_format(entity, state, 0);
+ format = v4l2_subdev_state_get_format(state, 0);
code->code = format->code;
mutex_unlock(&entity->lock);
}
@@ -276,7 +259,7 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
if (!state)
return -EINVAL;
- format = vsp1_entity_get_pad_format(entity, state, fse->pad);
+ format = v4l2_subdev_state_get_format(state, fse->pad);
mutex_lock(&entity->lock);
@@ -346,7 +329,7 @@ int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
goto done;
}
- format = vsp1_entity_get_pad_format(entity, state, fmt->pad);
+ format = v4l2_subdev_state_get_format(state, fmt->pad);
if (fmt->pad == entity->source_pad) {
/* The output format can't be modified. */
@@ -374,7 +357,7 @@ int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
fmt->format = *format;
/* Propagate the format to the source pad. */
- format = vsp1_entity_get_pad_format(entity, state, entity->source_pad);
+ format = v4l2_subdev_state_get_format(state, entity->source_pad);
*format = fmt->format;
/* Reset the crop and compose rectangles. */
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index 735f32dde4b5..e913befe7fc8 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -138,10 +138,6 @@ struct v4l2_subdev_state *
vsp1_entity_get_state(struct vsp1_entity *entity,
struct v4l2_subdev_state *sd_state,
enum v4l2_subdev_format_whence which);
-struct v4l2_mbus_framefmt *
-vsp1_entity_get_pad_format(struct vsp1_entity *entity,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad);
struct v4l2_rect *
vsp1_entity_get_pad_selection(struct vsp1_entity *entity,
struct v4l2_subdev_state *sd_state,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_histo.c b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
index cd1c8778662e..0d7e4afae0f8 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
@@ -229,8 +229,7 @@ static int histo_get_selection(struct v4l2_subdev *subdev,
case V4L2_SEL_TGT_CROP_BOUNDS:
case V4L2_SEL_TGT_CROP_DEFAULT:
- format = vsp1_entity_get_pad_format(&histo->entity, state,
- HISTO_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, HISTO_PAD_SINK);
sel->r.left = 0;
sel->r.top = 0;
sel->r.width = format->width;
@@ -262,8 +261,7 @@ static int histo_set_crop(struct v4l2_subdev *subdev,
struct v4l2_rect *selection;
/* The crop rectangle must be inside the input frame. */
- format = vsp1_entity_get_pad_format(&histo->entity, sd_state,
- HISTO_PAD_SINK);
+ format = v4l2_subdev_state_get_format(sd_state, HISTO_PAD_SINK);
sel->r.left = clamp_t(unsigned int, sel->r.left, 0, format->width - 1);
sel->r.top = clamp_t(unsigned int, sel->r.top, 0, format->height - 1);
sel->r.width = clamp_t(unsigned int, sel->r.width, HISTO_MIN_SIZE,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
index bc1299c29ac9..4a8cce808c93 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
@@ -78,7 +78,7 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
goto done;
}
- format = vsp1_entity_get_pad_format(&hsit->entity, state, fmt->pad);
+ format = v4l2_subdev_state_get_format(state, fmt->pad);
if (fmt->pad == HSIT_PAD_SOURCE) {
/*
@@ -101,8 +101,7 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
fmt->format = *format;
/* Propagate the format to the source pad. */
- format = vsp1_entity_get_pad_format(&hsit->entity, state,
- HSIT_PAD_SOURCE);
+ format = v4l2_subdev_state_get_format(state, HSIT_PAD_SOURCE);
*format = fmt->format;
format->code = hsit->inverse ? MEDIA_BUS_FMT_ARGB8888_1X32
: MEDIA_BUS_FMT_AHSV8888_1X32;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
index b1d21a54837b..29d4c1521e6a 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
@@ -93,8 +93,8 @@ static void lif_configure_stream(struct vsp1_entity *entity,
unsigned int obth;
unsigned int lbth;
- format = vsp1_entity_get_pad_format(&lif->entity, lif->entity.state,
- LIF_PAD_SOURCE);
+ format = v4l2_subdev_state_get_format(lif->entity.state,
+ LIF_PAD_SOURCE);
switch (entity->vsp1->version & VI6_IP_VERSION_MODEL_MASK) {
case VI6_IP_VERSION_MODEL_VSPD_GEN2:
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index c47579efc65f..19d9f078748c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -80,12 +80,10 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_PSTRIDE, pstride);
/* Format */
- sink_format = vsp1_entity_get_pad_format(&rpf->entity,
- rpf->entity.state,
- RWPF_PAD_SINK);
- source_format = vsp1_entity_get_pad_format(&rpf->entity,
- rpf->entity.state,
- RWPF_PAD_SOURCE);
+ sink_format = v4l2_subdev_state_get_format(rpf->entity.state,
+ RWPF_PAD_SINK);
+ source_format = v4l2_subdev_state_get_format(rpf->entity.state,
+ RWPF_PAD_SOURCE);
infmt = VI6_RPF_INFMT_CIPM
| (fmtinfo->hwfmt << VI6_RPF_INFMT_RDFMT_SHIFT);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index 09fb6ffa14e2..574623a48a3d 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -79,7 +79,7 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
fmt->format.code != MEDIA_BUS_FMT_AYUV8_1X32)
fmt->format.code = MEDIA_BUS_FMT_AYUV8_1X32;
- format = vsp1_entity_get_pad_format(&rwpf->entity, state, fmt->pad);
+ format = v4l2_subdev_state_get_format(state, fmt->pad);
if (fmt->pad == RWPF_PAD_SOURCE) {
/*
@@ -113,8 +113,7 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
}
/* Propagate the format to the source pad. */
- format = vsp1_entity_get_pad_format(&rwpf->entity, state,
- RWPF_PAD_SOURCE);
+ format = v4l2_subdev_state_get_format(state, RWPF_PAD_SOURCE);
*format = fmt->format;
if (rwpf->flip.rotate) {
@@ -157,8 +156,7 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
break;
case V4L2_SEL_TGT_CROP_BOUNDS:
- format = vsp1_entity_get_pad_format(&rwpf->entity, state,
- RWPF_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
sel->r.left = 0;
sel->r.top = 0;
sel->r.width = format->width;
@@ -204,8 +202,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
}
/* Make sure the crop rectangle is entirely contained in the image. */
- format = vsp1_entity_get_pad_format(&rwpf->entity, state,
- RWPF_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
/*
* Restrict the crop rectangle coordinates to multiples of 2 to avoid
@@ -229,8 +226,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
*crop = sel->r;
/* Propagate the format to the source pad. */
- format = vsp1_entity_get_pad_format(&rwpf->entity, state,
- RWPF_PAD_SOURCE);
+ format = v4l2_subdev_state_get_format(state, RWPF_PAD_SOURCE);
format->width = crop->width;
format->height = crop->height;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
index 11e008aa9f20..749ba7705000 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -131,7 +131,7 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
if (!state)
return -EINVAL;
- format = vsp1_entity_get_pad_format(&sru->entity, state, SRU_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, SRU_PAD_SINK);
mutex_lock(&sru->entity.lock);
@@ -184,8 +184,7 @@ static void sru_try_format(struct vsp1_sru *sru,
case SRU_PAD_SOURCE:
/* The SRU can't perform format conversion. */
- format = vsp1_entity_get_pad_format(&sru->entity, sd_state,
- SRU_PAD_SINK);
+ format = v4l2_subdev_state_get_format(sd_state, SRU_PAD_SINK);
fmt->code = format->code;
/*
@@ -234,13 +233,12 @@ static int sru_set_format(struct v4l2_subdev *subdev,
sru_try_format(sru, state, fmt->pad, &fmt->format);
- format = vsp1_entity_get_pad_format(&sru->entity, state, fmt->pad);
+ format = v4l2_subdev_state_get_format(state, fmt->pad);
*format = fmt->format;
if (fmt->pad == SRU_PAD_SINK) {
/* Propagate the format to the source pad. */
- format = vsp1_entity_get_pad_format(&sru->entity, state,
- SRU_PAD_SOURCE);
+ format = v4l2_subdev_state_get_format(state, SRU_PAD_SOURCE);
*format = fmt->format;
sru_try_format(sru, state, SRU_PAD_SOURCE, format);
@@ -277,10 +275,9 @@ static void sru_configure_stream(struct vsp1_entity *entity,
struct v4l2_mbus_framefmt *output;
u32 ctrl0;
- input = vsp1_entity_get_pad_format(&sru->entity, sru->entity.state,
- SRU_PAD_SINK);
- output = vsp1_entity_get_pad_format(&sru->entity, sru->entity.state,
- SRU_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(sru->entity.state, SRU_PAD_SINK);
+ output = v4l2_subdev_state_get_format(sru->entity.state,
+ SRU_PAD_SOURCE);
if (input->code == MEDIA_BUS_FMT_ARGB8888_1X32)
ctrl0 = VI6_SRU_CTRL0_PARAM2 | VI6_SRU_CTRL0_PARAM3
@@ -307,10 +304,9 @@ static unsigned int sru_max_width(struct vsp1_entity *entity,
struct v4l2_mbus_framefmt *input;
struct v4l2_mbus_framefmt *output;
- input = vsp1_entity_get_pad_format(&sru->entity, sru->entity.state,
- SRU_PAD_SINK);
- output = vsp1_entity_get_pad_format(&sru->entity, sru->entity.state,
- SRU_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(sru->entity.state, SRU_PAD_SINK);
+ output = v4l2_subdev_state_get_format(sru->entity.state,
+ SRU_PAD_SOURCE);
/*
* The maximum input width of the SRU is 288 input pixels, but 32
@@ -333,10 +329,9 @@ static void sru_partition(struct vsp1_entity *entity,
struct v4l2_mbus_framefmt *input;
struct v4l2_mbus_framefmt *output;
- input = vsp1_entity_get_pad_format(&sru->entity, sru->entity.state,
- SRU_PAD_SINK);
- output = vsp1_entity_get_pad_format(&sru->entity, sru->entity.state,
- SRU_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(sru->entity.state, SRU_PAD_SINK);
+ output = v4l2_subdev_state_get_format(sru->entity.state,
+ SRU_PAD_SOURCE);
/* Adapt if SRUx2 is enabled. */
if (input->width != output->width) {
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index d89f1197b86c..887b1f70611a 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -136,7 +136,7 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
if (!state)
return -EINVAL;
- format = vsp1_entity_get_pad_format(&uds->entity, state, UDS_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, UDS_PAD_SINK);
mutex_lock(&uds->entity.lock);
@@ -183,8 +183,7 @@ static void uds_try_format(struct vsp1_uds *uds,
case UDS_PAD_SOURCE:
/* The UDS scales but can't perform format conversion. */
- format = vsp1_entity_get_pad_format(&uds->entity, sd_state,
- UDS_PAD_SINK);
+ format = v4l2_subdev_state_get_format(sd_state, UDS_PAD_SINK);
fmt->code = format->code;
uds_output_limits(format->width, &minimum, &maximum);
@@ -217,13 +216,12 @@ static int uds_set_format(struct v4l2_subdev *subdev,
uds_try_format(uds, state, fmt->pad, &fmt->format);
- format = vsp1_entity_get_pad_format(&uds->entity, state, fmt->pad);
+ format = v4l2_subdev_state_get_format(state, fmt->pad);
*format = fmt->format;
if (fmt->pad == UDS_PAD_SINK) {
/* Propagate the format to the source pad. */
- format = vsp1_entity_get_pad_format(&uds->entity, state,
- UDS_PAD_SOURCE);
+ format = v4l2_subdev_state_get_format(state, UDS_PAD_SOURCE);
*format = fmt->format;
uds_try_format(uds, state, UDS_PAD_SOURCE, format);
@@ -265,10 +263,9 @@ static void uds_configure_stream(struct vsp1_entity *entity,
unsigned int vscale;
bool multitap;
- input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.state,
- UDS_PAD_SINK);
- output = vsp1_entity_get_pad_format(&uds->entity, uds->entity.state,
- UDS_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(uds->entity.state, UDS_PAD_SINK);
+ output = v4l2_subdev_state_get_format(uds->entity.state,
+ UDS_PAD_SOURCE);
hscale = uds_compute_ratio(input->width, output->width);
vscale = uds_compute_ratio(input->height, output->height);
@@ -310,8 +307,8 @@ static void uds_configure_partition(struct vsp1_entity *entity,
struct vsp1_partition *partition = pipe->partition;
const struct v4l2_mbus_framefmt *output;
- output = vsp1_entity_get_pad_format(&uds->entity, uds->entity.state,
- UDS_PAD_SOURCE);
+ output = v4l2_subdev_state_get_format(uds->entity.state,
+ UDS_PAD_SOURCE);
/* Input size clipping. */
vsp1_uds_write(uds, dlb, VI6_UDS_HSZCLIP, VI6_UDS_HSZCLIP_HCEN |
@@ -335,10 +332,9 @@ static unsigned int uds_max_width(struct vsp1_entity *entity,
const struct v4l2_mbus_framefmt *input;
unsigned int hscale;
- input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.state,
- UDS_PAD_SINK);
- output = vsp1_entity_get_pad_format(&uds->entity, uds->entity.state,
- UDS_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(uds->entity.state, UDS_PAD_SINK);
+ output = v4l2_subdev_state_get_format(uds->entity.state,
+ UDS_PAD_SOURCE);
hscale = output->width / input->width;
/*
@@ -377,10 +373,9 @@ static void uds_partition(struct vsp1_entity *entity,
partition->uds_sink = *window;
partition->uds_source = *window;
- input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.state,
- UDS_PAD_SINK);
- output = vsp1_entity_get_pad_format(&uds->entity, uds->entity.state,
- UDS_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(uds->entity.state, UDS_PAD_SINK);
+ output = v4l2_subdev_state_get_format(uds->entity.state,
+ UDS_PAD_SOURCE);
partition->uds_sink.width = window->width * input->width
/ output->width;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uif.c b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
index f66936a28a2a..ee5b6ba22898 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
@@ -104,8 +104,7 @@ static int uif_get_selection(struct v4l2_subdev *subdev,
switch (sel->target) {
case V4L2_SEL_TGT_CROP_BOUNDS:
case V4L2_SEL_TGT_CROP_DEFAULT:
- format = vsp1_entity_get_pad_format(&uif->entity, state,
- UIF_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, UIF_PAD_SINK);
sel->r.left = 0;
sel->r.top = 0;
sel->r.width = format->width;
@@ -150,7 +149,7 @@ static int uif_set_selection(struct v4l2_subdev *subdev,
}
/* The crop rectangle must be inside the input frame. */
- format = vsp1_entity_get_pad_format(&uif->entity, state, UIF_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, UIF_PAD_SINK);
sel->r.left = clamp_t(unsigned int, sel->r.left, 0, format->width - 1);
sel->r.top = clamp_t(unsigned int, sel->r.top, 0, format->height - 1);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index d6f2739456bf..813699d372f6 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -203,9 +203,8 @@ static void vsp1_video_calculate_partition(struct vsp1_pipeline *pipe,
* Partitions are computed on the size before rotation, use the format
* at the WPF sink.
*/
- format = vsp1_entity_get_pad_format(&pipe->output->entity,
- pipe->output->entity.state,
- RWPF_PAD_SINK);
+ format = v4l2_subdev_state_get_format(pipe->output->entity.state,
+ RWPF_PAD_SINK);
/* A single partition simply processes the output size in full. */
if (pipe->partitions <= 1) {
@@ -268,9 +267,8 @@ static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
* Partitions are computed on the size before rotation, use the format
* at the WPF sink.
*/
- format = vsp1_entity_get_pad_format(&pipe->output->entity,
- pipe->output->entity.state,
- RWPF_PAD_SINK);
+ format = v4l2_subdev_state_get_format(pipe->output->entity.state,
+ RWPF_PAD_SINK);
div_size = format->width;
/*
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index 9693aeab1cac..5129181b8217 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -65,12 +65,10 @@ static int vsp1_wpf_set_rotation(struct vsp1_rwpf *wpf, unsigned int rotation)
goto done;
}
- sink_format = vsp1_entity_get_pad_format(&wpf->entity,
- wpf->entity.state,
- RWPF_PAD_SINK);
- source_format = vsp1_entity_get_pad_format(&wpf->entity,
- wpf->entity.state,
- RWPF_PAD_SOURCE);
+ sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
+ RWPF_PAD_SINK);
+ source_format = v4l2_subdev_state_get_format(wpf->entity.state,
+ RWPF_PAD_SOURCE);
mutex_lock(&wpf->entity.lock);
@@ -245,12 +243,10 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
u32 srcrpf = 0;
int ret;
- sink_format = vsp1_entity_get_pad_format(&wpf->entity,
- wpf->entity.state,
- RWPF_PAD_SINK);
- source_format = vsp1_entity_get_pad_format(&wpf->entity,
- wpf->entity.state,
- RWPF_PAD_SOURCE);
+ sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
+ RWPF_PAD_SINK);
+ source_format = v4l2_subdev_state_get_format(wpf->entity.state,
+ RWPF_PAD_SOURCE);
/* Format */
if (!pipe->lif || wpf->writeback) {
@@ -383,9 +379,8 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
unsigned int flip;
unsigned int i;
- sink_format = vsp1_entity_get_pad_format(&wpf->entity,
- wpf->entity.state,
- RWPF_PAD_SINK);
+ sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
+ RWPF_PAD_SINK);
width = sink_format->width;
height = sink_format->height;
left = 0;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 02/19] media: renesas: vsp1: Drop vsp1_entity_get_pad_selection() wrapper
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 01/19] media: renesas: vsp1: Drop vsp1_entity_get_pad_format() wrapper Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 03/19] media: renesas: vsp1: Drop vsp1_rwpf_get_crop() wrapper Laurent Pinchart
` (17 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham, Laurent Pinchart
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The vsp1_entity_get_pad_selection() function is just a wrapper around
v4l2_subdev_state_get_crop() or v4l2_subdev_state_get_compose() without
any added value. Drop it and call the functions it wraps directly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_entity.c | 32 ++-----------------
.../media/platform/renesas/vsp1/vsp1_entity.h | 4 ---
.../media/platform/renesas/vsp1/vsp1_hgo.c | 7 ++--
.../media/platform/renesas/vsp1/vsp1_hgt.c | 7 ++--
.../media/platform/renesas/vsp1/vsp1_histo.c | 31 ++++++------------
.../media/platform/renesas/vsp1/vsp1_rpf.c | 6 ++--
.../media/platform/renesas/vsp1/vsp1_uif.c | 9 ++----
7 files changed, 20 insertions(+), 76 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index fa748cf89d44..8d39f1ee00ab 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -127,32 +127,6 @@ vsp1_entity_get_state(struct vsp1_entity *entity,
}
}
-/**
- * vsp1_entity_get_pad_selection - Get a pad selection from storage for entity
- * @entity: the entity
- * @sd_state: the state storage
- * @pad: the pad number
- * @target: the selection target
- *
- * Return the selection rectangle stored in the given configuration for an
- * entity's pad. The configuration can be an ACTIVE or TRY configuration. The
- * selection target can be COMPOSE or CROP.
- */
-struct v4l2_rect *
-vsp1_entity_get_pad_selection(struct vsp1_entity *entity,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, unsigned int target)
-{
- switch (target) {
- case V4L2_SEL_TGT_COMPOSE:
- return v4l2_subdev_state_get_compose(sd_state, pad);
- case V4L2_SEL_TGT_CROP:
- return v4l2_subdev_state_get_crop(sd_state, pad);
- default:
- return NULL;
- }
-}
-
/*
* vsp1_subdev_get_pad_format - Subdev pad get_fmt handler
* @subdev: V4L2 subdevice
@@ -361,15 +335,13 @@ int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
*format = fmt->format;
/* Reset the crop and compose rectangles. */
- selection = vsp1_entity_get_pad_selection(entity, state, fmt->pad,
- V4L2_SEL_TGT_CROP);
+ selection = v4l2_subdev_state_get_crop(state, fmt->pad);
selection->left = 0;
selection->top = 0;
selection->width = format->width;
selection->height = format->height;
- selection = vsp1_entity_get_pad_selection(entity, state, fmt->pad,
- V4L2_SEL_TGT_COMPOSE);
+ selection = v4l2_subdev_state_get_compose(state, fmt->pad);
selection->left = 0;
selection->top = 0;
selection->width = format->width;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index e913befe7fc8..802c0c2acab0 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -138,10 +138,6 @@ struct v4l2_subdev_state *
vsp1_entity_get_state(struct vsp1_entity *entity,
struct v4l2_subdev_state *sd_state,
enum v4l2_subdev_format_whence which);
-struct v4l2_rect *
-vsp1_entity_get_pad_selection(struct vsp1_entity *entity,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad, unsigned int target);
void vsp1_entity_route_setup(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
index 40c571a987ef..4ee5f0e5e9c3 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
@@ -140,11 +140,8 @@ static void hgo_configure_stream(struct vsp1_entity *entity,
unsigned int hratio;
unsigned int vratio;
- crop = vsp1_entity_get_pad_selection(entity, entity->state,
- HISTO_PAD_SINK, V4L2_SEL_TGT_CROP);
- compose = vsp1_entity_get_pad_selection(entity, entity->state,
- HISTO_PAD_SINK,
- V4L2_SEL_TGT_COMPOSE);
+ crop = v4l2_subdev_state_get_crop(entity->state, HISTO_PAD_SINK);
+ compose = v4l2_subdev_state_get_compose(entity->state, HISTO_PAD_SINK);
vsp1_hgo_write(hgo, dlb, VI6_HGO_REGRST, VI6_HGO_REGRST_RCLEA);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
index 8281b86874ab..b739d8045576 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
@@ -139,11 +139,8 @@ static void hgt_configure_stream(struct vsp1_entity *entity,
u8 upper;
unsigned int i;
- crop = vsp1_entity_get_pad_selection(entity, entity->state,
- HISTO_PAD_SINK, V4L2_SEL_TGT_CROP);
- compose = vsp1_entity_get_pad_selection(entity, entity->state,
- HISTO_PAD_SINK,
- V4L2_SEL_TGT_COMPOSE);
+ crop = v4l2_subdev_state_get_crop(entity->state, HISTO_PAD_SINK);
+ compose = v4l2_subdev_state_get_compose(entity->state, HISTO_PAD_SINK);
vsp1_hgt_write(hgt, dlb, VI6_HGT_REGRST, VI6_HGT_REGRST_RCLEA);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_histo.c b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
index 0d7e4afae0f8..85d2fc538327 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
@@ -218,9 +218,7 @@ static int histo_get_selection(struct v4l2_subdev *subdev,
switch (sel->target) {
case V4L2_SEL_TGT_COMPOSE_BOUNDS:
case V4L2_SEL_TGT_COMPOSE_DEFAULT:
- crop = vsp1_entity_get_pad_selection(&histo->entity, state,
- HISTO_PAD_SINK,
- V4L2_SEL_TGT_CROP);
+ crop = v4l2_subdev_state_get_crop(state, HISTO_PAD_SINK);
sel->r.left = 0;
sel->r.top = 0;
sel->r.width = crop->width;
@@ -237,9 +235,11 @@ static int histo_get_selection(struct v4l2_subdev *subdev,
break;
case V4L2_SEL_TGT_COMPOSE:
+ sel->r = *v4l2_subdev_state_get_compose(state, sel->pad);
+ break;
+
case V4L2_SEL_TGT_CROP:
- sel->r = *vsp1_entity_get_pad_selection(&histo->entity, state,
- sel->pad, sel->target);
+ sel->r = *v4l2_subdev_state_get_crop(state, sel->pad);
break;
default:
@@ -256,9 +256,7 @@ static int histo_set_crop(struct v4l2_subdev *subdev,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_selection *sel)
{
- struct vsp1_histogram *histo = subdev_to_histo(subdev);
struct v4l2_mbus_framefmt *format;
- struct v4l2_rect *selection;
/* The crop rectangle must be inside the input frame. */
format = v4l2_subdev_state_get_format(sd_state, HISTO_PAD_SINK);
@@ -270,14 +268,8 @@ static int histo_set_crop(struct v4l2_subdev *subdev,
format->height - sel->r.top);
/* Set the crop rectangle and reset the compose rectangle. */
- selection = vsp1_entity_get_pad_selection(&histo->entity, sd_state,
- sel->pad, V4L2_SEL_TGT_CROP);
- *selection = sel->r;
-
- selection = vsp1_entity_get_pad_selection(&histo->entity, sd_state,
- sel->pad,
- V4L2_SEL_TGT_COMPOSE);
- *selection = sel->r;
+ *v4l2_subdev_state_get_crop(sd_state, sel->pad) = sel->r;
+ *v4l2_subdev_state_get_compose(sd_state, sel->pad) = sel->r;
return 0;
}
@@ -286,7 +278,6 @@ static int histo_set_compose(struct v4l2_subdev *subdev,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_selection *sel)
{
- struct vsp1_histogram *histo = subdev_to_histo(subdev);
struct v4l2_rect *compose;
struct v4l2_rect *crop;
unsigned int ratio;
@@ -299,9 +290,7 @@ static int histo_set_compose(struct v4l2_subdev *subdev,
sel->r.left = 0;
sel->r.top = 0;
- crop = vsp1_entity_get_pad_selection(&histo->entity, sd_state,
- sel->pad,
- V4L2_SEL_TGT_CROP);
+ crop = v4l2_subdev_state_get_crop(sd_state, sel->pad);
/*
* Clamp the width and height to acceptable values first and then
@@ -326,9 +315,7 @@ static int histo_set_compose(struct v4l2_subdev *subdev,
ratio = 1 << (crop->height * 2 / sel->r.height / 3);
sel->r.height = crop->height / ratio;
- compose = vsp1_entity_get_pad_selection(&histo->entity, sd_state,
- sel->pad,
- V4L2_SEL_TGT_COMPOSE);
+ compose = v4l2_subdev_state_get_compose(sd_state, sel->pad);
*compose = sel->r;
return 0;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 19d9f078748c..4efcec5253d6 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -155,10 +155,8 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
if (pipe->brx) {
const struct v4l2_rect *compose;
- compose = vsp1_entity_get_pad_selection(pipe->brx,
- pipe->brx->state,
- rpf->brx_input,
- V4L2_SEL_TGT_COMPOSE);
+ compose = v4l2_subdev_state_get_compose(pipe->brx->state,
+ rpf->brx_input);
left = compose->left;
top = compose->top;
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uif.c b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
index ee5b6ba22898..cecd2f7024f4 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
@@ -112,8 +112,7 @@ static int uif_get_selection(struct v4l2_subdev *subdev,
break;
case V4L2_SEL_TGT_CROP:
- sel->r = *vsp1_entity_get_pad_selection(&uif->entity, state,
- sel->pad, sel->target);
+ sel->r = *v4l2_subdev_state_get_crop(state, sel->pad);
break;
default:
@@ -159,8 +158,7 @@ static int uif_set_selection(struct v4l2_subdev *subdev,
format->height - sel->r.top);
/* Store the crop rectangle. */
- selection = vsp1_entity_get_pad_selection(&uif->entity, state,
- sel->pad, V4L2_SEL_TGT_CROP);
+ selection = v4l2_subdev_state_get_crop(state, sel->pad);
*selection = sel->r;
done:
@@ -202,8 +200,7 @@ static void uif_configure_stream(struct vsp1_entity *entity,
vsp1_uif_write(uif, dlb, VI6_UIF_DISCOM_DOCMPMR,
VI6_UIF_DISCOM_DOCMPMR_SEL(9));
- crop = vsp1_entity_get_pad_selection(entity, entity->state,
- UIF_PAD_SINK, V4L2_SEL_TGT_CROP);
+ crop = v4l2_subdev_state_get_crop(entity->state, UIF_PAD_SINK);
left = crop->left;
width = crop->width;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 03/19] media: renesas: vsp1: Drop vsp1_rwpf_get_crop() wrapper
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 01/19] media: renesas: vsp1: Drop vsp1_entity_get_pad_format() wrapper Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 02/19] media: renesas: vsp1: Drop vsp1_entity_get_pad_selection() wrapper Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 04/19] media: renesas: vsp1: Drop brx_get_compose() wrapper Laurent Pinchart
` (16 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham, Laurent Pinchart
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The vsp1_rwpf_get_crop() function is just a wrapper around
v4l2_subdev_state_get_crop() without any added value. Drop it and call
v4l2_subdev_state_get_crop() directly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
drivers/media/platform/renesas/vsp1/vsp1_rpf.c | 2 +-
drivers/media/platform/renesas/vsp1/vsp1_rwpf.c | 12 +++---------
drivers/media/platform/renesas/vsp1/vsp1_rwpf.h | 3 ---
3 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 4efcec5253d6..3e62638fdce6 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -298,7 +298,7 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
* offsets are needed, as planes 2 and 3 always have identical
* strides.
*/
- crop = *vsp1_rwpf_get_crop(rpf, rpf->entity.state);
+ crop = *v4l2_subdev_state_get_crop(rpf->entity.state, RWPF_PAD_SINK);
/*
* Partition Algorithm Control
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index 574623a48a3d..9d38203e73d0 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -16,12 +16,6 @@
#define RWPF_MIN_WIDTH 1
#define RWPF_MIN_HEIGHT 1
-struct v4l2_rect *vsp1_rwpf_get_crop(struct vsp1_rwpf *rwpf,
- struct v4l2_subdev_state *sd_state)
-{
- return v4l2_subdev_state_get_crop(sd_state, RWPF_PAD_SINK);
-}
-
/* -----------------------------------------------------------------------------
* V4L2 Subdevice Operations
*/
@@ -105,7 +99,7 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
struct v4l2_rect *crop;
/* Update the sink crop rectangle. */
- crop = vsp1_rwpf_get_crop(rwpf, state);
+ crop = v4l2_subdev_state_get_crop(state, RWPF_PAD_SINK);
crop->left = 0;
crop->top = 0;
crop->width = fmt->format.width;
@@ -152,7 +146,7 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
switch (sel->target) {
case V4L2_SEL_TGT_CROP:
- sel->r = *vsp1_rwpf_get_crop(rwpf, state);
+ sel->r = *v4l2_subdev_state_get_crop(state, RWPF_PAD_SINK);
break;
case V4L2_SEL_TGT_CROP_BOUNDS:
@@ -222,7 +216,7 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
sel->r.height = min_t(unsigned int, sel->r.height,
format->height - sel->r.top);
- crop = vsp1_rwpf_get_crop(rwpf, state);
+ crop = v4l2_subdev_state_get_crop(state, RWPF_PAD_SINK);
*crop = sel->r;
/* Propagate the format to the source pad. */
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
index e0d212c70b2f..5ac9f0a6fafc 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
@@ -85,7 +85,4 @@ int vsp1_rwpf_init_ctrls(struct vsp1_rwpf *rwpf, unsigned int ncontrols);
extern const struct v4l2_subdev_ops vsp1_rwpf_subdev_ops;
-struct v4l2_rect *vsp1_rwpf_get_crop(struct vsp1_rwpf *rwpf,
- struct v4l2_subdev_state *sd_state);
-
#endif /* __VSP1_RWPF_H__ */
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 04/19] media: renesas: vsp1: Drop brx_get_compose() wrapper
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (2 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 03/19] media: renesas: vsp1: Drop vsp1_rwpf_get_crop() wrapper Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 05/19] media: renesas: vsp1: Drop custom .get_fmt() handler for histogram Laurent Pinchart
` (15 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham, Laurent Pinchart
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The brx_get_compose() function is just a wrapper around
v4l2_subdev_state_get_compose() without any added value. Drop it and
call v4l2_subdev_state_get_compose() directly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
drivers/media/platform/renesas/vsp1/vsp1_brx.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index 0eb4d8fe4285..05940d0427bf 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -96,13 +96,6 @@ static int brx_enum_frame_size(struct v4l2_subdev *subdev,
return 0;
}
-static struct v4l2_rect *brx_get_compose(struct vsp1_brx *brx,
- struct v4l2_subdev_state *sd_state,
- unsigned int pad)
-{
- return v4l2_subdev_state_get_compose(sd_state, pad);
-}
-
static void brx_try_format(struct vsp1_brx *brx,
struct v4l2_subdev_state *sd_state,
unsigned int pad, struct v4l2_mbus_framefmt *fmt)
@@ -157,7 +150,7 @@ static int brx_set_format(struct v4l2_subdev *subdev,
if (fmt->pad != brx->entity.source_pad) {
struct v4l2_rect *compose;
- compose = brx_get_compose(brx, state, fmt->pad);
+ compose = v4l2_subdev_state_get_compose(state, fmt->pad);
compose->left = 0;
compose->top = 0;
compose->width = format->width;
@@ -204,7 +197,7 @@ static int brx_get_selection(struct v4l2_subdev *subdev,
return -EINVAL;
mutex_lock(&brx->entity.lock);
- sel->r = *brx_get_compose(brx, state, sel->pad);
+ sel->r = *v4l2_subdev_state_get_compose(state, sel->pad);
mutex_unlock(&brx->entity.lock);
return 0;
@@ -253,7 +246,7 @@ static int brx_set_selection(struct v4l2_subdev *subdev,
sel->r.width = format->width;
sel->r.height = format->height;
- compose = brx_get_compose(brx, state, sel->pad);
+ compose = v4l2_subdev_state_get_compose(state, sel->pad);
*compose = sel->r;
done:
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 05/19] media: renesas: vsp1: Drop custom .get_fmt() handler for histogram
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (3 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 04/19] media: renesas: vsp1: Drop brx_get_compose() wrapper Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 06/19] media: renesas: vsp1: Move partition calculation to vsp1_pipe.c Laurent Pinchart
` (14 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham, Laurent Pinchart
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The histogram module is the only one that has a custom .get_fmt()
handler, to handle the special case of the output format being fixed.
This can equally well be handled in the .set_fmt() handler instead.
Beside avoiding special cases and using the same .get_fmt() handler in
all modules, it ensures that the correct format is stored in the active
state for the source pad, including when .set_fmt() is called from
vsp1_entity_init_state(). Both are needed to later switch to the V4L2
subdev active state API.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_histo.c | 29 +++++++------------
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_histo.c b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
index 85d2fc538327..9c2d4c91bfad 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
@@ -352,30 +352,21 @@ static int histo_set_selection(struct v4l2_subdev *subdev,
return ret;
}
-static int histo_get_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt)
-{
- if (fmt->pad == HISTO_PAD_SOURCE) {
- fmt->format.code = MEDIA_BUS_FMT_FIXED;
- fmt->format.width = 0;
- fmt->format.height = 0;
- fmt->format.field = V4L2_FIELD_NONE;
- fmt->format.colorspace = V4L2_COLORSPACE_RAW;
- return 0;
- }
-
- return vsp1_subdev_get_pad_format(subdev, sd_state, fmt);
-}
-
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_SINK)
- return histo_get_format(subdev, sd_state, fmt);
+ if (fmt->pad == HISTO_PAD_SOURCE) {
+ fmt->format.code = MEDIA_BUS_FMT_FIXED;
+ fmt->format.width = 0;
+ fmt->format.height = 0;
+ fmt->format.field = V4L2_FIELD_NONE;
+ fmt->format.colorspace = V4L2_COLORSPACE_RAW;
+
+ return 0;
+ }
return vsp1_subdev_set_pad_format(subdev, sd_state, fmt,
histo->formats, histo->num_formats,
@@ -386,7 +377,7 @@ static int histo_set_format(struct v4l2_subdev *subdev,
static const struct v4l2_subdev_pad_ops histo_pad_ops = {
.enum_mbus_code = histo_enum_mbus_code,
.enum_frame_size = histo_enum_frame_size,
- .get_fmt = histo_get_format,
+ .get_fmt = vsp1_subdev_get_pad_format,
.set_fmt = histo_set_format,
.get_selection = histo_get_selection,
.set_selection = histo_set_selection,
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 06/19] media: renesas: vsp1: Move partition calculation to vsp1_pipe.c
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (4 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 05/19] media: renesas: vsp1: Drop custom .get_fmt() handler for histogram Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 07/19] media: renesas: vsp1: Simplify partition calculation Laurent Pinchart
` (13 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
The partition calculation code, located in vsp1_video.c, is not specific
to video pipelines. To prepare for its usage in DRM pipelines, move it
to vsp1_pipe.c.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
Changes since v1:
- Drop stray \t in comment
---
.../media/platform/renesas/vsp1/vsp1_pipe.c | 85 ++++++++-
.../media/platform/renesas/vsp1/vsp1_pipe.h | 6 +-
.../media/platform/renesas/vsp1/vsp1_video.c | 169 +++++-------------
3 files changed, 130 insertions(+), 130 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
index 68d05243c3ee..fc66dcfdb838 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
@@ -444,6 +444,10 @@ void vsp1_pipeline_propagate_alpha(struct vsp1_pipeline *pipe,
vsp1_uds_set_alpha(pipe->uds, dlb, alpha);
}
+/* -----------------------------------------------------------------------------
+ * VSP1 Partition Algorithm support
+ */
+
/*
* Propagate the partition calculations through the pipeline
*
@@ -452,10 +456,10 @@ void vsp1_pipeline_propagate_alpha(struct vsp1_pipeline *pipe,
* source. Each entity must produce the partition required for the previous
* entity in the pipeline.
*/
-void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe,
- struct vsp1_partition *partition,
- unsigned int index,
- struct vsp1_partition_window *window)
+static void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int index,
+ struct vsp1_partition_window *window)
{
struct vsp1_entity *entity;
@@ -466,3 +470,76 @@ void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe,
}
}
+/*
+ * vsp1_pipeline_calculate_partition - Calculate pipeline configuration for a
+ * partition
+ *
+ * @pipe: the pipeline
+ * @partition: partition that will hold the calculated values
+ * @div_size: pre-determined maximum partition division size
+ * @index: partition index
+ */
+void vsp1_pipeline_calculate_partition(struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int div_size,
+ unsigned int index)
+{
+ const struct v4l2_mbus_framefmt *format;
+ struct vsp1_partition_window window;
+ unsigned int modulus;
+
+ /*
+ * Partitions are computed on the size before rotation, use the format
+ * at the WPF sink.
+ */
+ format = v4l2_subdev_state_get_format(pipe->output->entity.state,
+ RWPF_PAD_SINK);
+
+ /* A single partition simply processes the output size in full. */
+ if (pipe->partitions <= 1) {
+ window.left = 0;
+ window.width = format->width;
+
+ vsp1_pipeline_propagate_partition(pipe, partition, index,
+ &window);
+ return;
+ }
+
+ /* Initialise the partition with sane starting conditions. */
+ window.left = index * div_size;
+ window.width = div_size;
+
+ modulus = format->width % div_size;
+
+ /*
+ * We need to prevent the last partition from being smaller than the
+ * *minimum* width of the hardware capabilities.
+ *
+ * If the modulus is less than half of the partition size,
+ * the penultimate partition is reduced to half, which is added
+ * to the final partition: |1234|1234|1234|12|341|
+ * to prevent this: |1234|1234|1234|1234|1|.
+ */
+ if (modulus) {
+ /*
+ * pipe->partitions is 1 based, whilst index is a 0 based index.
+ * Normalise this locally.
+ */
+ unsigned int partitions = pipe->partitions - 1;
+
+ if (modulus < div_size / 2) {
+ if (index == partitions - 1) {
+ /* Halve the penultimate partition. */
+ window.width = div_size / 2;
+ } else if (index == partitions) {
+ /* Increase the final partition. */
+ window.width = (div_size / 2) + modulus;
+ window.left -= div_size / 2;
+ }
+ } else if (index == partitions) {
+ window.width = modulus;
+ }
+ }
+
+ vsp1_pipeline_propagate_partition(pipe, partition, index, &window);
+}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
index 674b5748d929..02e98d843730 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
@@ -166,10 +166,10 @@ void vsp1_pipeline_propagate_alpha(struct vsp1_pipeline *pipe,
struct vsp1_dl_body *dlb,
unsigned int alpha);
-void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe,
+void vsp1_pipeline_calculate_partition(struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
- unsigned int index,
- struct vsp1_partition_window *window);
+ unsigned int div_size,
+ unsigned int index);
const struct vsp1_format_info *vsp1_get_format_info(struct vsp1_device *vsp1,
u32 fourcc);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 813699d372f6..6dbae2bf9b14 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -178,129 +178,6 @@ static int __vsp1_video_try_format(struct vsp1_video *video,
return 0;
}
-/* -----------------------------------------------------------------------------
- * VSP1 Partition Algorithm support
- */
-
-/**
- * vsp1_video_calculate_partition - Calculate the active partition output window
- *
- * @pipe: the pipeline
- * @partition: partition that will hold the calculated values
- * @div_size: pre-determined maximum partition division size
- * @index: partition index
- */
-static void vsp1_video_calculate_partition(struct vsp1_pipeline *pipe,
- struct vsp1_partition *partition,
- unsigned int div_size,
- unsigned int index)
-{
- const struct v4l2_mbus_framefmt *format;
- struct vsp1_partition_window window;
- unsigned int modulus;
-
- /*
- * Partitions are computed on the size before rotation, use the format
- * at the WPF sink.
- */
- format = v4l2_subdev_state_get_format(pipe->output->entity.state,
- RWPF_PAD_SINK);
-
- /* A single partition simply processes the output size in full. */
- if (pipe->partitions <= 1) {
- window.left = 0;
- window.width = format->width;
-
- vsp1_pipeline_propagate_partition(pipe, partition, index,
- &window);
- return;
- }
-
- /* Initialise the partition with sane starting conditions. */
- window.left = index * div_size;
- window.width = div_size;
-
- modulus = format->width % div_size;
-
- /*
- * We need to prevent the last partition from being smaller than the
- * *minimum* width of the hardware capabilities.
- *
- * If the modulus is less than half of the partition size,
- * the penultimate partition is reduced to half, which is added
- * to the final partition: |1234|1234|1234|12|341|
- * to prevent this: |1234|1234|1234|1234|1|.
- */
- if (modulus) {
- /*
- * pipe->partitions is 1 based, whilst index is a 0 based index.
- * Normalise this locally.
- */
- unsigned int partitions = pipe->partitions - 1;
-
- if (modulus < div_size / 2) {
- if (index == partitions - 1) {
- /* Halve the penultimate partition. */
- window.width = div_size / 2;
- } else if (index == partitions) {
- /* Increase the final partition. */
- window.width = (div_size / 2) + modulus;
- window.left -= div_size / 2;
- }
- } else if (index == partitions) {
- window.width = modulus;
- }
- }
-
- vsp1_pipeline_propagate_partition(pipe, partition, index, &window);
-}
-
-static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
-{
- struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
- const struct v4l2_mbus_framefmt *format;
- struct vsp1_entity *entity;
- unsigned int div_size;
- unsigned int i;
-
- /*
- * Partitions are computed on the size before rotation, use the format
- * at the WPF sink.
- */
- format = v4l2_subdev_state_get_format(pipe->output->entity.state,
- RWPF_PAD_SINK);
- div_size = format->width;
-
- /*
- * Only Gen3+ hardware requires image partitioning, Gen2 will operate
- * with a single partition that covers the whole output.
- */
- if (vsp1->info->gen >= 3) {
- list_for_each_entry(entity, &pipe->entities, list_pipe) {
- unsigned int entity_max;
-
- if (!entity->ops->max_width)
- continue;
-
- entity_max = entity->ops->max_width(entity, pipe);
- if (entity_max)
- div_size = min(div_size, entity_max);
- }
- }
-
- pipe->partitions = DIV_ROUND_UP(format->width, div_size);
- pipe->part_table = kcalloc(pipe->partitions, sizeof(*pipe->part_table),
- GFP_KERNEL);
- if (!pipe->part_table)
- return -ENOMEM;
-
- for (i = 0; i < pipe->partitions; ++i)
- vsp1_video_calculate_partition(pipe, &pipe->part_table[i],
- div_size, i);
-
- return 0;
-}
-
/* -----------------------------------------------------------------------------
* Pipeline Management
*/
@@ -788,6 +665,52 @@ static void vsp1_video_buffer_queue(struct vb2_buffer *vb)
spin_unlock_irqrestore(&pipe->irqlock, flags);
}
+static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
+{
+ struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
+ const struct v4l2_mbus_framefmt *format;
+ struct vsp1_entity *entity;
+ unsigned int div_size;
+ unsigned int i;
+
+ /*
+ * Partitions are computed on the size before rotation, use the format
+ * at the WPF sink.
+ */
+ format = v4l2_subdev_state_get_format(pipe->output->entity.state,
+ RWPF_PAD_SINK);
+ div_size = format->width;
+
+ /*
+ * Only Gen3+ hardware requires image partitioning, Gen2 will operate
+ * with a single partition that covers the whole output.
+ */
+ if (vsp1->info->gen >= 3) {
+ list_for_each_entry(entity, &pipe->entities, list_pipe) {
+ unsigned int entity_max;
+
+ if (!entity->ops->max_width)
+ continue;
+
+ entity_max = entity->ops->max_width(entity, pipe);
+ if (entity_max)
+ div_size = min(div_size, entity_max);
+ }
+ }
+
+ pipe->partitions = DIV_ROUND_UP(format->width, div_size);
+ pipe->part_table = kcalloc(pipe->partitions, sizeof(*pipe->part_table),
+ GFP_KERNEL);
+ if (!pipe->part_table)
+ return -ENOMEM;
+
+ for (i = 0; i < pipe->partitions; ++i)
+ vsp1_pipeline_calculate_partition(pipe, &pipe->part_table[i],
+ div_size, i);
+
+ return 0;
+}
+
static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
{
struct vsp1_entity *entity;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 07/19] media: renesas: vsp1: Simplify partition calculation
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (5 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 06/19] media: renesas: vsp1: Move partition calculation to vsp1_pipe.c Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 08/19] media: renesas: vsp1: Store RPF partition configuration per RPF instance Laurent Pinchart
` (12 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
When calculation a partition in vsp1_pipeline_calculate_partition(),
there is no need to handle the case where the whole image is covered by
a single partition locally. In that case, the index and div_size
parameters are 0 and format->width respectively, which makes the general
code behave exactly as the special case. Drop the special case.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
drivers/media/platform/renesas/vsp1/vsp1_pipe.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
index fc66dcfdb838..ac52ed8f65ba 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
@@ -495,16 +495,6 @@ void vsp1_pipeline_calculate_partition(struct vsp1_pipeline *pipe,
format = v4l2_subdev_state_get_format(pipe->output->entity.state,
RWPF_PAD_SINK);
- /* A single partition simply processes the output size in full. */
- if (pipe->partitions <= 1) {
- window.left = 0;
- window.width = format->width;
-
- vsp1_pipeline_propagate_partition(pipe, partition, index,
- &window);
- return;
- }
-
/* Initialise the partition with sane starting conditions. */
window.left = index * div_size;
window.width = div_size;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 08/19] media: renesas: vsp1: Store RPF partition configuration per RPF instance
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (6 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 07/19] media: renesas: vsp1: Simplify partition calculation Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 09/19] media: renesas: vsp1: Pass partition pointer to .configure_partition() Laurent Pinchart
` (11 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
The vsp1_partition structure stores the RPF partition configuration in a
single field for all RPF instances, while each RPF can have its own
configuration. Fix it by storing the configuration separately for each
RPF instance.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Fixes: ab45e8585182 ("media: v4l: vsp1: Allow entities to participate in the partition algorithm")
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
Changes since v1:
- Add Fixes: tag
---
drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 +-
drivers/media/platform/renesas/vsp1/vsp1_rpf.c | 8 +++++---
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
index 02e98d843730..840fd3288efb 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
@@ -73,7 +73,7 @@ struct vsp1_partition_window {
* @wpf: The WPF partition window configuration
*/
struct vsp1_partition {
- struct vsp1_partition_window rpf;
+ struct vsp1_partition_window rpf[VSP1_MAX_RPF];
struct vsp1_partition_window uds_sink;
struct vsp1_partition_window uds_source;
struct vsp1_partition_window sru;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 3e62638fdce6..42b0c5655404 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -311,8 +311,8 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
* 'width' need to be adjusted.
*/
if (pipe->partitions > 1) {
- crop.width = pipe->partition->rpf.width;
- crop.left += pipe->partition->rpf.left;
+ crop.width = pipe->partition->rpf[rpf->entity.index].width;
+ crop.left += pipe->partition->rpf[rpf->entity.index].left;
}
if (pipe->interlaced) {
@@ -367,7 +367,9 @@ static void rpf_partition(struct vsp1_entity *entity,
unsigned int partition_idx,
struct vsp1_partition_window *window)
{
- partition->rpf = *window;
+ struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
+
+ partition->rpf[rpf->entity.index] = *window;
}
static const struct vsp1_entity_operations rpf_entity_ops = {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 09/19] media: renesas: vsp1: Pass partition pointer to .configure_partition()
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (7 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 08/19] media: renesas: vsp1: Store RPF partition configuration per RPF instance Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 10/19] media: renesas: vsp1: Replace vsp1_partition_window with v4l2_rect Laurent Pinchart
` (10 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
The entity .configure_partition() function operates on a partition, and
has to retrieve that partition from the pipeline's current partition
field. Pass the partition pointer to the function to make it clearer
what partition it operates on, and remove the vsp1_pipeline.partition
field.
This change clearly shows that the DRM pipeline doesn't use partitions,
which makes entity implementation more complex and error-prone. This
will be addressed in a further cleanup.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
drivers/media/platform/renesas/vsp1/vsp1_drm.c | 2 +-
drivers/media/platform/renesas/vsp1/vsp1_entity.c | 4 +++-
drivers/media/platform/renesas/vsp1/vsp1_entity.h | 2 ++
drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 --
drivers/media/platform/renesas/vsp1/vsp1_rpf.c | 5 +++--
drivers/media/platform/renesas/vsp1/vsp1_uds.c | 2 +-
drivers/media/platform/renesas/vsp1/vsp1_video.c | 5 ++---
drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 5 +++--
8 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index 9b087bd8df7d..3954c138fa7b 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -569,7 +569,7 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
vsp1_entity_route_setup(entity, pipe, dlb);
vsp1_entity_configure_stream(entity, pipe, dl, dlb);
vsp1_entity_configure_frame(entity, pipe, dl, dlb);
- vsp1_entity_configure_partition(entity, pipe, dl, dlb);
+ vsp1_entity_configure_partition(entity, pipe, NULL, dl, dlb);
}
vsp1_dl_list_commit(dl, dl_flags);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 8d39f1ee00ab..e9de75de8bde 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -89,11 +89,13 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity,
void vsp1_entity_configure_partition(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
+ const struct vsp1_partition *partition,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
{
if (entity->ops->configure_partition)
- entity->ops->configure_partition(entity, pipe, dl, dlb);
+ entity->ops->configure_partition(entity, pipe, partition,
+ dl, dlb);
}
/* -----------------------------------------------------------------------------
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index 802c0c2acab0..7b86b2fef3e5 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -85,6 +85,7 @@ struct vsp1_entity_operations {
struct vsp1_dl_list *, struct vsp1_dl_body *);
void (*configure_partition)(struct vsp1_entity *,
struct vsp1_pipeline *,
+ const struct vsp1_partition *,
struct vsp1_dl_list *,
struct vsp1_dl_body *);
unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *);
@@ -155,6 +156,7 @@ void vsp1_entity_configure_frame(struct vsp1_entity *entity,
void vsp1_entity_configure_partition(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
+ const struct vsp1_partition *partition,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
index 840fd3288efb..3d2e35ac8fa0 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
@@ -106,7 +106,6 @@ struct vsp1_partition {
* @configured: when false the @stream_config shall be written to the hardware
* @interlaced: True when the pipeline is configured in interlaced mode
* @partitions: The number of partitions used to process one frame
- * @partition: The current partition for configuration to process
* @part_table: The pre-calculated partitions used by the pipeline
*/
struct vsp1_pipeline {
@@ -146,7 +145,6 @@ struct vsp1_pipeline {
bool interlaced;
unsigned int partitions;
- struct vsp1_partition *partition;
struct vsp1_partition *part_table;
u32 underrun_count;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 42b0c5655404..3b8a62299226 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -280,6 +280,7 @@ static void rpf_configure_frame(struct vsp1_entity *entity,
static void rpf_configure_partition(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
+ const struct vsp1_partition *partition,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
{
@@ -311,8 +312,8 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
* 'width' need to be adjusted.
*/
if (pipe->partitions > 1) {
- crop.width = pipe->partition->rpf[rpf->entity.index].width;
- crop.left += pipe->partition->rpf[rpf->entity.index].left;
+ crop.width = partition->rpf[rpf->entity.index].width;
+ crop.left += partition->rpf[rpf->entity.index].left;
}
if (pipe->interlaced) {
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index 887b1f70611a..737362ca2315 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -300,11 +300,11 @@ static void uds_configure_stream(struct vsp1_entity *entity,
static void uds_configure_partition(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
+ const struct vsp1_partition *partition,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
{
struct vsp1_uds *uds = to_uds(&entity->subdev);
- struct vsp1_partition *partition = pipe->partition;
const struct v4l2_mbus_framefmt *output;
output = v4l2_subdev_state_get_format(uds->entity.state,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 6dbae2bf9b14..2c0e10df8f3e 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -246,13 +246,12 @@ static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
unsigned int partition)
{
+ struct vsp1_partition *part = &pipe->part_table[partition];
struct vsp1_dl_body *dlb = vsp1_dl_list_get_body0(dl);
struct vsp1_entity *entity;
- pipe->partition = &pipe->part_table[partition];
-
list_for_each_entry(entity, &pipe->entities, list_pipe)
- vsp1_entity_configure_partition(entity, pipe, dl, dlb);
+ vsp1_entity_configure_partition(entity, pipe, part, dl, dlb);
}
static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index 5129181b8217..80fe7571f4ff 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -363,6 +363,7 @@ static void wpf_configure_frame(struct vsp1_entity *entity,
static void wpf_configure_partition(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
+ const struct vsp1_partition *partition,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
{
@@ -390,8 +391,8 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
* multiple slices.
*/
if (pipe->partitions > 1) {
- width = pipe->partition->wpf.width;
- left = pipe->partition->wpf.left;
+ width = partition->wpf.width;
+ left = partition->wpf.left;
}
vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 10/19] media: renesas: vsp1: Replace vsp1_partition_window with v4l2_rect
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (8 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 09/19] media: renesas: vsp1: Pass partition pointer to .configure_partition() Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 11/19] media: renesas: vsp1: Add and use function to dump a pipeline to the log Laurent Pinchart
` (9 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
The vsp1_partition_window structure is used to store the horizontal size
of a partition window. This is all that is currently needed, as all
partitions span the whole image vertically. The horizontal window size
is retrieved in the .configure_partition() handler from the
vsp1_partition_window structure, and the vertical window size from the
subdev state.
Accessing the subdev state in the .configure_partition() handler is
problematic in the context of moving to the V4L2 subdev active state
API, as .configure_partition() is called in non-interruptable context,
and the state lock can't be taken. To avoid this, start by storing the
vertical size in the window, replacing the custom vsp1_partition_window
structure with a v4l2_rect. Retrieving the vertical size from the window
in .configure_partition() will be done in a subsequent change.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_entity.h | 3 +--
.../media/platform/renesas/vsp1/vsp1_pipe.c | 6 ++++--
.../media/platform/renesas/vsp1/vsp1_pipe.h | 21 +++++--------------
.../media/platform/renesas/vsp1/vsp1_rpf.c | 2 +-
.../media/platform/renesas/vsp1/vsp1_sru.c | 4 +++-
.../media/platform/renesas/vsp1/vsp1_uds.c | 10 ++++-----
.../media/platform/renesas/vsp1/vsp1_wpf.c | 2 +-
7 files changed, 20 insertions(+), 28 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index 7b86b2fef3e5..f67f60677644 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -19,7 +19,6 @@ struct vsp1_dl_body;
struct vsp1_dl_list;
struct vsp1_pipeline;
struct vsp1_partition;
-struct vsp1_partition_window;
enum vsp1_entity_type {
VSP1_ENTITY_BRS,
@@ -91,7 +90,7 @@ struct vsp1_entity_operations {
unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *);
void (*partition)(struct vsp1_entity *, struct vsp1_pipeline *,
struct vsp1_partition *, unsigned int,
- struct vsp1_partition_window *);
+ struct v4l2_rect *);
};
struct vsp1_entity {
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
index ac52ed8f65ba..f7701c5ff492 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
@@ -459,7 +459,7 @@ void vsp1_pipeline_propagate_alpha(struct vsp1_pipeline *pipe,
static void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int index,
- struct vsp1_partition_window *window)
+ struct v4l2_rect *window)
{
struct vsp1_entity *entity;
@@ -485,7 +485,7 @@ void vsp1_pipeline_calculate_partition(struct vsp1_pipeline *pipe,
unsigned int index)
{
const struct v4l2_mbus_framefmt *format;
- struct vsp1_partition_window window;
+ struct v4l2_rect window;
unsigned int modulus;
/*
@@ -498,6 +498,8 @@ void vsp1_pipeline_calculate_partition(struct vsp1_pipeline *pipe,
/* Initialise the partition with sane starting conditions. */
window.left = index * div_size;
window.width = div_size;
+ window.top = 0;
+ window.height = format->height;
modulus = format->width % div_size;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
index 3d2e35ac8fa0..c1f411227de7 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
@@ -53,17 +53,6 @@ enum vsp1_pipeline_state {
VSP1_PIPELINE_STOPPING,
};
-/*
- * struct vsp1_partition_window - Partition window coordinates
- * @left: horizontal coordinate of the partition start in pixels relative to the
- * left edge of the image
- * @width: partition width in pixels
- */
-struct vsp1_partition_window {
- unsigned int left;
- unsigned int width;
-};
-
/*
* struct vsp1_partition - A description of a slice for the partition algorithm
* @rpf: The RPF partition window configuration
@@ -73,11 +62,11 @@ struct vsp1_partition_window {
* @wpf: The WPF partition window configuration
*/
struct vsp1_partition {
- struct vsp1_partition_window rpf[VSP1_MAX_RPF];
- struct vsp1_partition_window uds_sink;
- struct vsp1_partition_window uds_source;
- struct vsp1_partition_window sru;
- struct vsp1_partition_window wpf;
+ struct v4l2_rect rpf[VSP1_MAX_RPF];
+ struct v4l2_rect uds_sink;
+ struct v4l2_rect uds_source;
+ struct v4l2_rect sru;
+ struct v4l2_rect wpf;
};
/*
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 3b8a62299226..862751616646 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -366,7 +366,7 @@ static void rpf_partition(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int partition_idx,
- struct vsp1_partition_window *window)
+ struct v4l2_rect *window)
{
struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
index 749ba7705000..8ce949de8d9a 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -323,7 +323,7 @@ static void sru_partition(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int partition_idx,
- struct vsp1_partition_window *window)
+ struct v4l2_rect *window)
{
struct vsp1_sru *sru = to_sru(&entity->subdev);
struct v4l2_mbus_framefmt *input;
@@ -337,6 +337,8 @@ static void sru_partition(struct vsp1_entity *entity,
if (input->width != output->width) {
window->width /= 2;
window->left /= 2;
+ window->height /= 2;
+ window->top /= 2;
}
partition->sru = *window;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index 737362ca2315..4a14fd3baac1 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -363,16 +363,12 @@ static void uds_partition(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int partition_idx,
- struct vsp1_partition_window *window)
+ struct v4l2_rect *window)
{
struct vsp1_uds *uds = to_uds(&entity->subdev);
const struct v4l2_mbus_framefmt *output;
const struct v4l2_mbus_framefmt *input;
- /* Initialise the partition state. */
- partition->uds_sink = *window;
- partition->uds_source = *window;
-
input = v4l2_subdev_state_get_format(uds->entity.state, UDS_PAD_SINK);
output = v4l2_subdev_state_get_format(uds->entity.state,
UDS_PAD_SOURCE);
@@ -381,6 +377,10 @@ static void uds_partition(struct vsp1_entity *entity,
/ output->width;
partition->uds_sink.left = window->left * input->width
/ output->width;
+ partition->uds_sink.height = input->height;
+ partition->uds_sink.top = 0;
+
+ partition->uds_source = *window;
*window = partition->uds_sink;
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index 80fe7571f4ff..f8d1e2f47691 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -515,7 +515,7 @@ static void wpf_partition(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int partition_idx,
- struct vsp1_partition_window *window)
+ struct v4l2_rect *window)
{
partition->wpf = *window;
}
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 11/19] media: renesas: vsp1: Add and use function to dump a pipeline to the log
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (9 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 10/19] media: renesas: vsp1: Replace vsp1_partition_window with v4l2_rect Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 18:59 ` [PATCH v2.1 " Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 12/19] media: renesas: vsp1: Keep the DRM pipeline entities sorted Laurent Pinchart
` (8 subsequent siblings)
19 siblings, 1 reply; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
It is useful for debugging purpose to dump a vsp1_pipeline to the kernel
log. Add a new function to do so, and use it when initializing the video
and DRM pipelines.
As __vsp1_pipeline_dump() needs to construct the log message
iteratively, it uses pr_cont(...) (exact equivalent to the more verbose
"printk(KERN_CONT ..."). The function thus can't use dev_dbg() to log
the initial part of the message, for two reasons:
- pr_cont() doesn't seem to work with dev_*(). Even if the format string
passed to dev_*() doesn't end with a '\n', pr_cont() starts a new line
in the log. This behaviour doesn't seem to be clearly documented, and
may or may not be on purpose.
- Messages printed by dev_dbg() may be omitted if dynamic debugging is
enabled. In that case, the continuation messages will still be
printed, leading to confusing log messages.
To still benefit from the dynamic debug infrastructure, we declare a
vsp1_pipeline_dump() macro that uses _dynamic_func_call() when dynamic
debugging is enabled. The whole vsp1_pipeline_dump() call can be
selected at runtime. The __vsp1_pipeline_dump() function then uses a
plain "printk(KERN_DEBUG ...)" to print the message header using the
debug log level, and pr_cont() to print the rest of the message on the
same line.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_drm.c | 5 +++++
.../media/platform/renesas/vsp1/vsp1_pipe.c | 22 +++++++++++++++++++
.../media/platform/renesas/vsp1/vsp1_pipe.h | 19 ++++++++++++++++
.../media/platform/renesas/vsp1/vsp1_video.c | 10 ++++++++-
4 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index 3954c138fa7b..1aa59a74672f 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -733,6 +733,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
if (ret < 0)
goto unlock;
+ vsp1_pipeline_dump(pipe, "LIF setup");
+
/* Enable the VSP1. */
ret = vsp1_device_get(vsp1);
if (ret < 0)
@@ -906,6 +908,9 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
}
vsp1_du_pipeline_setup_inputs(vsp1, pipe);
+
+ vsp1_pipeline_dump(pipe, "atomic update");
+
vsp1_du_pipeline_configure(pipe);
done:
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
index f7701c5ff492..b45e1b4eb5a1 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
@@ -301,6 +301,28 @@ void vsp1_pipeline_init(struct vsp1_pipeline *pipe)
pipe->state = VSP1_PIPELINE_STOPPED;
}
+void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe,
+ const char *msg)
+{
+ struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
+ struct vsp1_entity *entity;
+ bool first = true;
+
+ printk(KERN_DEBUG "%s: %s: pipe: ", dev_name(vsp1->dev), msg);
+
+ list_for_each_entry(entity, &pipe->entities, list_pipe) {
+ const char *name;
+
+ name = strchrnul(entity->subdev.name, ' ');
+ name = name ? name + 1 : entity->subdev.name;
+
+ pr_cont("%s%s", first ? "" : ", ", name);
+ first = false;
+ }
+
+ pr_cont("\n");
+}
+
/* Must be called with the pipe irqlock held. */
void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
{
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
index c1f411227de7..46a82a9f766a 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
@@ -9,6 +9,7 @@
#ifndef __VSP1_PIPE_H__
#define __VSP1_PIPE_H__
+#include <linux/dynamic_debug.h>
#include <linux/kref.h>
#include <linux/list.h>
#include <linux/spinlock.h>
@@ -142,6 +143,24 @@ struct vsp1_pipeline {
void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
void vsp1_pipeline_init(struct vsp1_pipeline *pipe);
+void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe,
+ const char *msg);
+
+#if defined(CONFIG_DYNAMIC_DEBUG) || \
+ (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
+#define vsp1_pipeline_dump(pipe, msg) \
+ _dynamic_func_call("vsp1_pipeline_dump()", __vsp1_pipeline_dump, pipe, msg)
+#elif defined(DEBUG)
+#define vsp1_pipeline_dump(pipe, msg) \
+ __vsp1_pipeline_dump(NULL, pipe, msg)
+#else
+#define vsp1_pipeline_dump(pipe, msg) \
+({ \
+ if (0) \
+ __vsp1_pipeline_dump(NULL, pipe, msg); \
+)}
+#endif
+
void vsp1_pipeline_run(struct vsp1_pipeline *pipe);
bool vsp1_pipeline_stopped(struct vsp1_pipeline *pipe);
int vsp1_pipeline_stop(struct vsp1_pipeline *pipe);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 2c0e10df8f3e..10a0485abc6c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -526,11 +526,19 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe,
static int vsp1_video_pipeline_init(struct vsp1_pipeline *pipe,
struct vsp1_video *video)
{
+ int ret;
+
vsp1_pipeline_init(pipe);
pipe->frame_end = vsp1_video_pipeline_frame_end;
- return vsp1_video_pipeline_build(pipe, video);
+ ret = vsp1_video_pipeline_build(pipe, video);
+ if (ret)
+ return ret;
+
+ vsp1_pipeline_dump(pipe, "video");
+
+ return 0;
}
static struct vsp1_pipeline *vsp1_video_pipeline_get(struct vsp1_video *video)
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2.1 11/19] media: renesas: vsp1: Add and use function to dump a pipeline to the log
2024-06-19 0:17 ` [PATCH v2 11/19] media: renesas: vsp1: Add and use function to dump a pipeline to the log Laurent Pinchart
@ 2024-06-19 18:59 ` Laurent Pinchart
0 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 18:59 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
It is useful for debugging purpose to dump a vsp1_pipeline to the kernel
log. Add a new function to do so, and use it when initializing the video
and DRM pipelines.
As __vsp1_pipeline_dump() needs to construct the log message
iteratively, it uses pr_cont(...) (exact equivalent to the more verbose
"printk(KERN_CONT ..."). The function thus can't use dev_dbg() to log
the initial part of the message, for two reasons:
- pr_cont() doesn't seem to work with dev_*(). Even if the format string
passed to dev_*() doesn't end with a '\n', pr_cont() starts a new line
in the log. This behaviour doesn't seem to be clearly documented, and
may or may not be on purpose.
- Messages printed by dev_dbg() may be omitted if dynamic debugging is
enabled. In that case, the continuation messages will still be
printed, leading to confusing log messages.
To still benefit from the dynamic debug infrastructure, we declare a
vsp1_pipeline_dump() macro that uses _dynamic_func_call() when dynamic
debugging is enabled. The whole vsp1_pipeline_dump() call can be
selected at runtime. The __vsp1_pipeline_dump() function then uses a
plain "printk(KERN_DEBUG ...)" to print the message header using the
debug log level, and pr_cont() to print the rest of the message on the
same line.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
Changes since v2:
- Name first parameter in __vsp1_pipeline_dump() definition
- Fix typo in !DEBUG vsp1_pipeline_dump() macro
---
.../media/platform/renesas/vsp1/vsp1_drm.c | 5 +++++
.../media/platform/renesas/vsp1/vsp1_pipe.c | 22 +++++++++++++++++++
.../media/platform/renesas/vsp1/vsp1_pipe.h | 19 ++++++++++++++++
.../media/platform/renesas/vsp1/vsp1_video.c | 10 ++++++++-
4 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index 3954c138fa7b..1aa59a74672f 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -733,6 +733,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
if (ret < 0)
goto unlock;
+ vsp1_pipeline_dump(pipe, "LIF setup");
+
/* Enable the VSP1. */
ret = vsp1_device_get(vsp1);
if (ret < 0)
@@ -906,6 +908,9 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
}
vsp1_du_pipeline_setup_inputs(vsp1, pipe);
+
+ vsp1_pipeline_dump(pipe, "atomic update");
+
vsp1_du_pipeline_configure(pipe);
done:
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
index f7701c5ff492..87cb62cf38fa 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
@@ -301,6 +301,28 @@ void vsp1_pipeline_init(struct vsp1_pipeline *pipe)
pipe->state = VSP1_PIPELINE_STOPPED;
}
+void __vsp1_pipeline_dump(struct _ddebug *dbg, struct vsp1_pipeline *pipe,
+ const char *msg)
+{
+ struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
+ struct vsp1_entity *entity;
+ bool first = true;
+
+ printk(KERN_DEBUG "%s: %s: pipe: ", dev_name(vsp1->dev), msg);
+
+ list_for_each_entry(entity, &pipe->entities, list_pipe) {
+ const char *name;
+
+ name = strchrnul(entity->subdev.name, ' ');
+ name = name ? name + 1 : entity->subdev.name;
+
+ pr_cont("%s%s", first ? "" : ", ", name);
+ first = false;
+ }
+
+ pr_cont("\n");
+}
+
/* Must be called with the pipe irqlock held. */
void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
{
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
index c1f411227de7..1ba7bdbad5a8 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
@@ -9,6 +9,7 @@
#ifndef __VSP1_PIPE_H__
#define __VSP1_PIPE_H__
+#include <linux/dynamic_debug.h>
#include <linux/kref.h>
#include <linux/list.h>
#include <linux/spinlock.h>
@@ -142,6 +143,24 @@ struct vsp1_pipeline {
void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
void vsp1_pipeline_init(struct vsp1_pipeline *pipe);
+void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe,
+ const char *msg);
+
+#if defined(CONFIG_DYNAMIC_DEBUG) || \
+ (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
+#define vsp1_pipeline_dump(pipe, msg) \
+ _dynamic_func_call("vsp1_pipeline_dump()", __vsp1_pipeline_dump, pipe, msg)
+#elif defined(DEBUG)
+#define vsp1_pipeline_dump(pipe, msg) \
+ __vsp1_pipeline_dump(NULL, pipe, msg)
+#else
+#define vsp1_pipeline_dump(pipe, msg) \
+({ \
+ if (0) \
+ __vsp1_pipeline_dump(NULL, pipe, msg); \
+})
+#endif
+
void vsp1_pipeline_run(struct vsp1_pipeline *pipe);
bool vsp1_pipeline_stopped(struct vsp1_pipeline *pipe);
int vsp1_pipeline_stop(struct vsp1_pipeline *pipe);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 2c0e10df8f3e..10a0485abc6c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -526,11 +526,19 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe,
static int vsp1_video_pipeline_init(struct vsp1_pipeline *pipe,
struct vsp1_video *video)
{
+ int ret;
+
vsp1_pipeline_init(pipe);
pipe->frame_end = vsp1_video_pipeline_frame_end;
- return vsp1_video_pipeline_build(pipe, video);
+ ret = vsp1_video_pipeline_build(pipe, video);
+ if (ret)
+ return ret;
+
+ vsp1_pipeline_dump(pipe, "video");
+
+ return 0;
}
static struct vsp1_pipeline *vsp1_video_pipeline_get(struct vsp1_video *video)
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 12/19] media: renesas: vsp1: Keep the DRM pipeline entities sorted
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (10 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 11/19] media: renesas: vsp1: Add and use function to dump a pipeline to the log Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 12:20 ` Kieran Bingham
2024-06-19 0:17 ` [PATCH v2 13/19] media: renesas: vsp1: Compute partitions for DRM pipelines Laurent Pinchart
` (7 subsequent siblings)
19 siblings, 1 reply; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
Some of the code that handles pipeline configuration assumes that
entities in a pipeline's entities list are sorted from sink to source.
To prepare for using that code with the DRM pipeline, insert the BRx
just before the WPF, and the RPFs at the head of the list.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/renesas/vsp1/vsp1_drm.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index 1aa59a74672f..e44359b661b6 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -317,7 +317,10 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device *vsp1,
list_add_tail(&released_brx->list_pipe,
&pipe->entities);
- /* Add the BRx to the pipeline. */
+ /*
+ * Add the BRx to the pipeline, inserting it just before the
+ * WPF.
+ */
dev_dbg(vsp1->dev, "%s: pipe %u: acquired %s\n",
__func__, pipe->lif->index, BRX_NAME(brx));
@@ -326,7 +329,8 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device *vsp1,
pipe->brx->sink = &pipe->output->entity;
pipe->brx->sink_pad = 0;
- list_add_tail(&pipe->brx->list_pipe, &pipe->entities);
+ list_add_tail(&pipe->brx->list_pipe,
+ &pipe->output->entity.list_pipe);
}
/*
@@ -420,7 +424,7 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1,
if (!rpf->entity.pipe) {
rpf->entity.pipe = pipe;
- list_add_tail(&rpf->entity.list_pipe, &pipe->entities);
+ list_add(&rpf->entity.list_pipe, &pipe->entities);
}
brx->inputs[i].rpf = rpf;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v2 12/19] media: renesas: vsp1: Keep the DRM pipeline entities sorted
2024-06-19 0:17 ` [PATCH v2 12/19] media: renesas: vsp1: Keep the DRM pipeline entities sorted Laurent Pinchart
@ 2024-06-19 12:20 ` Kieran Bingham
2024-06-19 12:31 ` Laurent Pinchart
0 siblings, 1 reply; 25+ messages in thread
From: Kieran Bingham @ 2024-06-19 12:20 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi
Quoting Laurent Pinchart (2024-06-19 01:17:15)
> Some of the code that handles pipeline configuration assumes that
> entities in a pipeline's entities list are sorted from sink to source.
> To prepare for using that code with the DRM pipeline, insert the BRx
> just before the WPF, and the RPFs at the head of the list.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/media/platform/renesas/vsp1/vsp1_drm.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> index 1aa59a74672f..e44359b661b6 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> @@ -317,7 +317,10 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device *vsp1,
> list_add_tail(&released_brx->list_pipe,
> &pipe->entities);
>
> - /* Add the BRx to the pipeline. */
> + /*
> + * Add the BRx to the pipeline, inserting it just before the
> + * WPF.
So - the pipe->output is from what I recall/can see the output wpf.
(struct vsp1_rwpf *output)
> + */
> dev_dbg(vsp1->dev, "%s: pipe %u: acquired %s\n",
> __func__, pipe->lif->index, BRX_NAME(brx));
>
> @@ -326,7 +329,8 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device *vsp1,
> pipe->brx->sink = &pipe->output->entity;
> pipe->brx->sink_pad = 0;
>
> - list_add_tail(&pipe->brx->list_pipe, &pipe->entities);
> + list_add_tail(&pipe->brx->list_pipe,
> + &pipe->output->entity.list_pipe);
But this reads to me as if we're adding the brx after ('the tail') of
the output WPF....
Now ... of course if we open up list_add_tail()
* Insert a new entry before the specified head.
And that checks out - because of course the list_add adds it as the
'next' item in the list... and we're using list_add_tail as a convenient
way to provide list_add_before() ...
So I believe this is correct, but the nuance of it reads back to front to me.
Because of that it possibly deserves a better comment to be explicit on
what it's doing, or makes me wonder if list.h should have something that
explicitly impliments
#define list_add_before list_add_tail
but otherwise - it does check out.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> }
>
> /*
> @@ -420,7 +424,7 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1,
>
> if (!rpf->entity.pipe) {
> rpf->entity.pipe = pipe;
> - list_add_tail(&rpf->entity.list_pipe, &pipe->entities);
> + list_add(&rpf->entity.list_pipe, &pipe->entities);
> }
>
> brx->inputs[i].rpf = rpf;
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 12/19] media: renesas: vsp1: Keep the DRM pipeline entities sorted
2024-06-19 12:20 ` Kieran Bingham
@ 2024-06-19 12:31 ` Laurent Pinchart
0 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 12:31 UTC (permalink / raw)
To: Kieran Bingham
Cc: linux-media, linux-renesas-soc, Sakari Ailus, Tomi Valkeinen,
Jacopo Mondi
On Wed, Jun 19, 2024 at 01:20:49PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-06-19 01:17:15)
> > Some of the code that handles pipeline configuration assumes that
> > entities in a pipeline's entities list are sorted from sink to source.
> > To prepare for using that code with the DRM pipeline, insert the BRx
> > just before the WPF, and the RPFs at the head of the list.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > drivers/media/platform/renesas/vsp1/vsp1_drm.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> > index 1aa59a74672f..e44359b661b6 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> > @@ -317,7 +317,10 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device *vsp1,
> > list_add_tail(&released_brx->list_pipe,
> > &pipe->entities);
> >
> > - /* Add the BRx to the pipeline. */
> > + /*
> > + * Add the BRx to the pipeline, inserting it just before the
> > + * WPF.
>
> So - the pipe->output is from what I recall/can see the output wpf.
> (struct vsp1_rwpf *output)
>
> > + */
> > dev_dbg(vsp1->dev, "%s: pipe %u: acquired %s\n",
> > __func__, pipe->lif->index, BRX_NAME(brx));
> >
> > @@ -326,7 +329,8 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device *vsp1,
> > pipe->brx->sink = &pipe->output->entity;
> > pipe->brx->sink_pad = 0;
> >
> > - list_add_tail(&pipe->brx->list_pipe, &pipe->entities);
> > + list_add_tail(&pipe->brx->list_pipe,
> > + &pipe->output->entity.list_pipe);
>
> But this reads to me as if we're adding the brx after ('the tail') of
> the output WPF....
>
> Now ... of course if we open up list_add_tail()
>
> * Insert a new entry before the specified head.
>
> And that checks out - because of course the list_add adds it as the
> 'next' item in the list... and we're using list_add_tail as a convenient
> way to provide list_add_before() ...
>
> So I believe this is correct, but the nuance of it reads back to front to me.
>
> Because of that it possibly deserves a better comment to be explicit on
> what it's doing, or makes me wonder if list.h should have something that
> explicitly impliments
>
> #define list_add_before list_add_tail
https://lore.kernel.org/all/alpine.LSU.2.11.1406061242370.16010@eggly.anvils/
I'll let you argue :-)
> but otherwise - it does check out.
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> > }
> >
> > /*
> > @@ -420,7 +424,7 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1,
> >
> > if (!rpf->entity.pipe) {
> > rpf->entity.pipe = pipe;
> > - list_add_tail(&rpf->entity.list_pipe, &pipe->entities);
> > + list_add(&rpf->entity.list_pipe, &pipe->entities);
> > }
> >
> > brx->inputs[i].rpf = rpf;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 13/19] media: renesas: vsp1: Compute partitions for DRM pipelines
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (11 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 12/19] media: renesas: vsp1: Keep the DRM pipeline entities sorted Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 14/19] media: renesas: vsp1: Get configuration from partition instead of state Laurent Pinchart
` (6 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
The DRM pipelines don't partition frames, as the hardware operates
synchronously with the display. The entity operations access
configuration data from the entity state in that case, instead of
accessing the partition structure. This requires special cases in
entity-specific code, increasing the driver complexity.
To prepare for simplifying the code, initialize a single partition for
the DRM pipelines, similarly to how video pipelines create one partition
spanning the full image when partitioning isn't needed. The partition is
allocated statically in the vsp1_drm_pipeline structure instead of
dynamically as for video pipelines, as DRM pipelines are guaranteed to
operate on a single partition.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
drivers/media/platform/renesas/vsp1/vsp1_drm.c | 9 ++++++++-
drivers/media/platform/renesas/vsp1/vsp1_drm.h | 2 ++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index e44359b661b6..11313e26a298 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -550,6 +550,9 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
struct vsp1_dl_body *dlb;
unsigned int dl_flags = 0;
+ vsp1_pipeline_calculate_partition(pipe, &pipe->part_table[0],
+ drm_pipe->width, 0);
+
if (drm_pipe->force_brx_release)
dl_flags |= VSP1_DL_FRAME_END_INTERNAL;
if (pipe->output->writeback)
@@ -573,7 +576,8 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
vsp1_entity_route_setup(entity, pipe, dlb);
vsp1_entity_configure_stream(entity, pipe, dl, dlb);
vsp1_entity_configure_frame(entity, pipe, dl, dlb);
- vsp1_entity_configure_partition(entity, pipe, NULL, dl, dlb);
+ vsp1_entity_configure_partition(entity, pipe,
+ &pipe->part_table[0], dl, dlb);
}
vsp1_dl_list_commit(dl, dl_flags);
@@ -968,6 +972,9 @@ int vsp1_drm_init(struct vsp1_device *vsp1)
vsp1_pipeline_init(pipe);
+ pipe->partitions = 1;
+ pipe->part_table = &drm_pipe->partition;
+
pipe->frame_end = vsp1_du_pipeline_frame_end;
/*
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.h b/drivers/media/platform/renesas/vsp1/vsp1_drm.h
index ab8b7e3161a2..3fd95b53f27e 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.h
@@ -20,6 +20,7 @@
/**
* struct vsp1_drm_pipeline - State for the API exposed to the DRM driver
* @pipe: the VSP1 pipeline used for display
+ * @partition: the pre-calculated partition used by the pipeline
* @width: output display width
* @height: output display height
* @force_brx_release: when set, release the BRx during the next reconfiguration
@@ -31,6 +32,7 @@
*/
struct vsp1_drm_pipeline {
struct vsp1_pipeline pipe;
+ struct vsp1_partition partition;
unsigned int width;
unsigned int height;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 14/19] media: renesas: vsp1: Get configuration from partition instead of state
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (12 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 13/19] media: renesas: vsp1: Compute partitions for DRM pipelines Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 15/19] media: renesas: vsp1: Name parameters to entity operations Laurent Pinchart
` (5 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
Entities access various piece of information from the subdev state when
configuring a partition. The same data is available through the
partition structure passed to the .configure_partition() operation. Use
it to avoid accessing the state, which will simplify moving to the V4L2
subdev active state API.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_rpf.c | 35 +++++++++----------
.../media/platform/renesas/vsp1/vsp1_uds.c | 6 +---
.../media/platform/renesas/vsp1/vsp1_wpf.c | 18 +++-------
3 files changed, 23 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 862751616646..b4558670b46f 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -289,7 +289,7 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
struct vsp1_device *vsp1 = rpf->entity.vsp1;
const struct vsp1_format_info *fmtinfo = rpf->fmtinfo;
const struct v4l2_pix_format_mplane *format = &rpf->format;
- struct v4l2_rect crop;
+ struct v4l2_rect crop = partition->rpf[rpf->entity.index];
/*
* Source size and crop offsets.
@@ -299,22 +299,6 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
* offsets are needed, as planes 2 and 3 always have identical
* strides.
*/
- crop = *v4l2_subdev_state_get_crop(rpf->entity.state, RWPF_PAD_SINK);
-
- /*
- * Partition Algorithm Control
- *
- * The partition algorithm can split this frame into multiple
- * slices. We must scale our partition window based on the pipe
- * configuration to match the destination partition window.
- * To achieve this, we adjust our crop to provide a 'sub-crop'
- * matching the expected partition window. Only 'left' and
- * 'width' need to be adjusted.
- */
- if (pipe->partitions > 1) {
- crop.width = partition->rpf[rpf->entity.index].width;
- crop.left += partition->rpf[rpf->entity.index].left;
- }
if (pipe->interlaced) {
crop.height = round_down(crop.height / 2, fmtinfo->vsub);
@@ -369,8 +353,23 @@ static void rpf_partition(struct vsp1_entity *entity,
struct v4l2_rect *window)
{
struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
+ struct v4l2_rect *rpf_rect = &partition->rpf[rpf->entity.index];
- partition->rpf[rpf->entity.index] = *window;
+ /*
+ * Partition Algorithm Control
+ *
+ * The partition algorithm can split this frame into multiple slices. We
+ * must adjust our partition window based on the pipe configuration to
+ * match the destination partition window. To achieve this, we adjust
+ * our crop to provide a 'sub-crop' matching the expected partition
+ * window.
+ */
+ *rpf_rect = *v4l2_subdev_state_get_crop(entity->state, RWPF_PAD_SINK);
+
+ if (pipe->partitions > 1) {
+ rpf_rect->width = window->width;
+ rpf_rect->left += window->left;
+ }
}
static const struct vsp1_entity_operations rpf_entity_ops = {
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index 4a14fd3baac1..e5953d86c17c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -305,10 +305,6 @@ static void uds_configure_partition(struct vsp1_entity *entity,
struct vsp1_dl_body *dlb)
{
struct vsp1_uds *uds = to_uds(&entity->subdev);
- const struct v4l2_mbus_framefmt *output;
-
- output = v4l2_subdev_state_get_format(uds->entity.state,
- UDS_PAD_SOURCE);
/* Input size clipping. */
vsp1_uds_write(uds, dlb, VI6_UDS_HSZCLIP, VI6_UDS_HSZCLIP_HCEN |
@@ -320,7 +316,7 @@ static void uds_configure_partition(struct vsp1_entity *entity,
vsp1_uds_write(uds, dlb, VI6_UDS_CLIP_SIZE,
(partition->uds_source.width
<< VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
- (output->height
+ (partition->uds_source.height
<< VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index f8d1e2f47691..5c363ff1d36c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -370,7 +370,6 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
struct vsp1_device *vsp1 = wpf->entity.vsp1;
struct vsp1_rwpf_memory mem = wpf->mem;
- const struct v4l2_mbus_framefmt *sink_format;
const struct v4l2_pix_format_mplane *format = &wpf->format;
const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
unsigned int width;
@@ -380,20 +379,13 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
unsigned int flip;
unsigned int i;
- sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
- RWPF_PAD_SINK);
- width = sink_format->width;
- height = sink_format->height;
- left = 0;
-
/*
- * Cropping. The partition algorithm can split the image into
- * multiple slices.
+ * Cropping. The partition algorithm can split the image into multiple
+ * slices.
*/
- if (pipe->partitions > 1) {
- width = partition->wpf.width;
- left = partition->wpf.left;
- }
+ width = partition->wpf.width;
+ left = partition->wpf.left;
+ height = partition->wpf.height;
vsp1_wpf_write(wpf, dlb, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
(0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 15/19] media: renesas: vsp1: Name parameters to entity operations
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (13 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 14/19] media: renesas: vsp1: Get configuration from partition instead of state Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 16/19] media: renesas: vsp1: Pass subdev state " Laurent Pinchart
` (4 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
checkpatch.pl complains when function arguments are not named:
WARNING: function definition argument 'struct vsp1_entity *' should also have an identifier name
+ void (*configure_stream)(struct vsp1_entity *,
In preparation for reworking some of the vsp1_entity_operations
functions, fix the warnings for the existing ones.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_entity.h | 35 +++++++++++--------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index f67f60677644..42000d6e2530 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -77,20 +77,27 @@ struct vsp1_route {
* configuration.
*/
struct vsp1_entity_operations {
- void (*destroy)(struct vsp1_entity *);
- void (*configure_stream)(struct vsp1_entity *, struct vsp1_pipeline *,
- struct vsp1_dl_list *, struct vsp1_dl_body *);
- void (*configure_frame)(struct vsp1_entity *, struct vsp1_pipeline *,
- struct vsp1_dl_list *, struct vsp1_dl_body *);
- void (*configure_partition)(struct vsp1_entity *,
- struct vsp1_pipeline *,
- const struct vsp1_partition *,
- struct vsp1_dl_list *,
- struct vsp1_dl_body *);
- unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *);
- void (*partition)(struct vsp1_entity *, struct vsp1_pipeline *,
- struct vsp1_partition *, unsigned int,
- struct v4l2_rect *);
+ void (*destroy)(struct vsp1_entity *entity);
+ void (*configure_stream)(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe,
+ struct vsp1_dl_list *dl,
+ struct vsp1_dl_body *dlb);
+ void (*configure_frame)(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe,
+ struct vsp1_dl_list *dl,
+ struct vsp1_dl_body *dlb);
+ void (*configure_partition)(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe,
+ const struct vsp1_partition *partition,
+ struct vsp1_dl_list *dl,
+ struct vsp1_dl_body *dlb);
+ unsigned int (*max_width)(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe);
+ void (*partition)(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int index,
+ struct v4l2_rect *window);
};
struct vsp1_entity {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 16/19] media: renesas: vsp1: Pass subdev state to entity operations
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (14 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 15/19] media: renesas: vsp1: Name parameters to entity operations Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 17/19] media: renesas: vsp1: Initialize control handler after subdev Laurent Pinchart
` (3 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham, Laurent Pinchart
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To prepare for the removal of the vsp1_entity.state field, pass the
state to all entity operations that needs to access it, instead of
accessing the state from the entity inside the operation handlers. This
lowers the number of accesses to the field.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
Changes since v1:
- Rename subdev state variables to 'state'
---
.../media/platform/renesas/vsp1/vsp1_brx.c | 4 ++--
.../media/platform/renesas/vsp1/vsp1_clu.c | 3 ++-
.../media/platform/renesas/vsp1/vsp1_drm.c | 3 ++-
.../media/platform/renesas/vsp1/vsp1_entity.c | 3 ++-
.../media/platform/renesas/vsp1/vsp1_entity.h | 4 ++++
.../media/platform/renesas/vsp1/vsp1_hgo.c | 5 +++--
.../media/platform/renesas/vsp1/vsp1_hgt.c | 5 +++--
.../media/platform/renesas/vsp1/vsp1_hsit.c | 1 +
.../media/platform/renesas/vsp1/vsp1_lif.c | 4 ++--
.../media/platform/renesas/vsp1/vsp1_lut.c | 1 +
.../media/platform/renesas/vsp1/vsp1_pipe.c | 4 ++--
.../media/platform/renesas/vsp1/vsp1_rpf.c | 10 +++++-----
.../media/platform/renesas/vsp1/vsp1_sru.c | 20 +++++++++----------
.../media/platform/renesas/vsp1/vsp1_uds.c | 20 +++++++++----------
.../media/platform/renesas/vsp1/vsp1_uif.c | 3 ++-
.../media/platform/renesas/vsp1/vsp1_video.c | 6 ++++--
.../media/platform/renesas/vsp1/vsp1_wpf.c | 9 +++++----
17 files changed, 58 insertions(+), 47 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index 05940d0427bf..5dee0490c593 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -272,6 +272,7 @@ static const struct v4l2_subdev_ops brx_ops = {
*/
static void brx_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -281,8 +282,7 @@ static void brx_configure_stream(struct vsp1_entity *entity,
unsigned int flags;
unsigned int i;
- format = v4l2_subdev_state_get_format(brx->entity.state,
- brx->entity.source_pad);
+ format = v4l2_subdev_state_get_format(state, brx->entity.source_pad);
/*
* The hardware is extremely flexible but we have no userspace API to
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_clu.c b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
index 1e57676a420c..98645bd2a983 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
@@ -170,6 +170,7 @@ static const struct v4l2_subdev_ops clu_ops = {
*/
static void clu_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -181,7 +182,7 @@ static void clu_configure_stream(struct vsp1_entity *entity,
* The yuv_mode can't be changed during streaming. Cache it internally
* for future runtime configuration calls.
*/
- format = v4l2_subdev_state_get_format(clu->entity.state, CLU_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, CLU_PAD_SINK);
clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index 11313e26a298..b5d1f238f7be 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -574,7 +574,8 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
}
vsp1_entity_route_setup(entity, pipe, dlb);
- vsp1_entity_configure_stream(entity, pipe, dl, dlb);
+ vsp1_entity_configure_stream(entity, entity->state, pipe,
+ dl, dlb);
vsp1_entity_configure_frame(entity, pipe, dl, dlb);
vsp1_entity_configure_partition(entity, pipe,
&pipe->part_table[0], dl, dlb);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index e9de75de8bde..8b8945bd8f10 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -70,12 +70,13 @@ void vsp1_entity_route_setup(struct vsp1_entity *entity,
}
void vsp1_entity_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
{
if (entity->ops->configure_stream)
- entity->ops->configure_stream(entity, pipe, dl, dlb);
+ entity->ops->configure_stream(entity, state, pipe, dl, dlb);
}
void vsp1_entity_configure_frame(struct vsp1_entity *entity,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index 42000d6e2530..1bcc9e27dfdc 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -79,6 +79,7 @@ struct vsp1_route {
struct vsp1_entity_operations {
void (*destroy)(struct vsp1_entity *entity);
void (*configure_stream)(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb);
@@ -92,8 +93,10 @@ struct vsp1_entity_operations {
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb);
unsigned int (*max_width)(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe);
void (*partition)(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int index,
@@ -151,6 +154,7 @@ void vsp1_entity_route_setup(struct vsp1_entity *entity,
struct vsp1_dl_body *dlb);
void vsp1_entity_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
index 4ee5f0e5e9c3..0ea87014a701 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
@@ -130,6 +130,7 @@ static const struct v4l2_ctrl_config hgo_num_bins_control = {
*/
static void hgo_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -140,8 +141,8 @@ static void hgo_configure_stream(struct vsp1_entity *entity,
unsigned int hratio;
unsigned int vratio;
- crop = v4l2_subdev_state_get_crop(entity->state, HISTO_PAD_SINK);
- compose = v4l2_subdev_state_get_compose(entity->state, HISTO_PAD_SINK);
+ crop = v4l2_subdev_state_get_crop(state, HISTO_PAD_SINK);
+ compose = v4l2_subdev_state_get_compose(state, HISTO_PAD_SINK);
vsp1_hgo_write(hgo, dlb, VI6_HGO_REGRST, VI6_HGO_REGRST_RCLEA);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
index b739d8045576..496354c8df0e 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
@@ -126,6 +126,7 @@ static const struct v4l2_ctrl_config hgt_hue_areas = {
*/
static void hgt_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -139,8 +140,8 @@ static void hgt_configure_stream(struct vsp1_entity *entity,
u8 upper;
unsigned int i;
- crop = v4l2_subdev_state_get_crop(entity->state, HISTO_PAD_SINK);
- compose = v4l2_subdev_state_get_compose(entity->state, HISTO_PAD_SINK);
+ crop = v4l2_subdev_state_get_crop(state, HISTO_PAD_SINK);
+ compose = v4l2_subdev_state_get_compose(state, HISTO_PAD_SINK);
vsp1_hgt_write(hgt, dlb, VI6_HGT_REGRST, VI6_HGT_REGRST_RCLEA);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
index 4a8cce808c93..8ba2a7c7305c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
@@ -127,6 +127,7 @@ static const struct v4l2_subdev_ops hsit_ops = {
*/
static void hsit_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
index 29d4c1521e6a..b3d83d1c5306 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
@@ -83,6 +83,7 @@ static const struct v4l2_subdev_ops lif_ops = {
*/
static void lif_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -93,8 +94,7 @@ static void lif_configure_stream(struct vsp1_entity *entity,
unsigned int obth;
unsigned int lbth;
- format = v4l2_subdev_state_get_format(lif->entity.state,
- LIF_PAD_SOURCE);
+ format = v4l2_subdev_state_get_format(state, LIF_PAD_SOURCE);
switch (entity->vsp1->version & VI6_IP_VERSION_MODEL_MASK) {
case VI6_IP_VERSION_MODEL_VSPD_GEN2:
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lut.c b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
index 451d24ab0b56..dd264e6532e0 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
@@ -146,6 +146,7 @@ static const struct v4l2_subdev_ops lut_ops = {
*/
static void lut_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
index b45e1b4eb5a1..b392115595f2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
@@ -487,8 +487,8 @@ static void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe,
list_for_each_entry_reverse(entity, &pipe->entities, list_pipe) {
if (entity->ops->partition)
- entity->ops->partition(entity, pipe, partition, index,
- window);
+ entity->ops->partition(entity, entity->state, pipe,
+ partition, index, window);
}
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index b4558670b46f..5c8b3ba1bd3c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -48,6 +48,7 @@ static inline void vsp1_rpf_write(struct vsp1_rwpf *rpf,
*/
static void rpf_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -80,10 +81,8 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_PSTRIDE, pstride);
/* Format */
- sink_format = v4l2_subdev_state_get_format(rpf->entity.state,
- RWPF_PAD_SINK);
- source_format = v4l2_subdev_state_get_format(rpf->entity.state,
- RWPF_PAD_SOURCE);
+ sink_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
+ source_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SOURCE);
infmt = VI6_RPF_INFMT_CIPM
| (fmtinfo->hwfmt << VI6_RPF_INFMT_RDFMT_SHIFT);
@@ -347,6 +346,7 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
}
static void rpf_partition(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int partition_idx,
@@ -364,7 +364,7 @@ static void rpf_partition(struct vsp1_entity *entity,
* our crop to provide a 'sub-crop' matching the expected partition
* window.
*/
- *rpf_rect = *v4l2_subdev_state_get_crop(entity->state, RWPF_PAD_SINK);
+ *rpf_rect = *v4l2_subdev_state_get_crop(state, RWPF_PAD_SINK);
if (pipe->partitions > 1) {
rpf_rect->width = window->width;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
index 8ce949de8d9a..1759ce642e6e 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -265,6 +265,7 @@ static const struct v4l2_subdev_ops sru_ops = {
*/
static void sru_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -275,9 +276,8 @@ static void sru_configure_stream(struct vsp1_entity *entity,
struct v4l2_mbus_framefmt *output;
u32 ctrl0;
- input = v4l2_subdev_state_get_format(sru->entity.state, SRU_PAD_SINK);
- output = v4l2_subdev_state_get_format(sru->entity.state,
- SRU_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(state, SRU_PAD_SINK);
+ output = v4l2_subdev_state_get_format(state, SRU_PAD_SOURCE);
if (input->code == MEDIA_BUS_FMT_ARGB8888_1X32)
ctrl0 = VI6_SRU_CTRL0_PARAM2 | VI6_SRU_CTRL0_PARAM3
@@ -298,15 +298,14 @@ static void sru_configure_stream(struct vsp1_entity *entity,
}
static unsigned int sru_max_width(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe)
{
- struct vsp1_sru *sru = to_sru(&entity->subdev);
struct v4l2_mbus_framefmt *input;
struct v4l2_mbus_framefmt *output;
- input = v4l2_subdev_state_get_format(sru->entity.state, SRU_PAD_SINK);
- output = v4l2_subdev_state_get_format(sru->entity.state,
- SRU_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(state, SRU_PAD_SINK);
+ output = v4l2_subdev_state_get_format(state, SRU_PAD_SOURCE);
/*
* The maximum input width of the SRU is 288 input pixels, but 32
@@ -320,18 +319,17 @@ static unsigned int sru_max_width(struct vsp1_entity *entity,
}
static void sru_partition(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int partition_idx,
struct v4l2_rect *window)
{
- struct vsp1_sru *sru = to_sru(&entity->subdev);
struct v4l2_mbus_framefmt *input;
struct v4l2_mbus_framefmt *output;
- input = v4l2_subdev_state_get_format(sru->entity.state, SRU_PAD_SINK);
- output = v4l2_subdev_state_get_format(sru->entity.state,
- SRU_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(state, SRU_PAD_SINK);
+ output = v4l2_subdev_state_get_format(state, SRU_PAD_SOURCE);
/* Adapt if SRUx2 is enabled. */
if (input->width != output->width) {
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index e5953d86c17c..c5a38478cf8c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -252,6 +252,7 @@ static const struct v4l2_subdev_ops uds_ops = {
*/
static void uds_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -263,9 +264,8 @@ static void uds_configure_stream(struct vsp1_entity *entity,
unsigned int vscale;
bool multitap;
- input = v4l2_subdev_state_get_format(uds->entity.state, UDS_PAD_SINK);
- output = v4l2_subdev_state_get_format(uds->entity.state,
- UDS_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(state, UDS_PAD_SINK);
+ output = v4l2_subdev_state_get_format(state, UDS_PAD_SOURCE);
hscale = uds_compute_ratio(input->width, output->width);
vscale = uds_compute_ratio(input->height, output->height);
@@ -321,16 +321,15 @@ static void uds_configure_partition(struct vsp1_entity *entity,
}
static unsigned int uds_max_width(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe)
{
- struct vsp1_uds *uds = to_uds(&entity->subdev);
const struct v4l2_mbus_framefmt *output;
const struct v4l2_mbus_framefmt *input;
unsigned int hscale;
- input = v4l2_subdev_state_get_format(uds->entity.state, UDS_PAD_SINK);
- output = v4l2_subdev_state_get_format(uds->entity.state,
- UDS_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(state, UDS_PAD_SINK);
+ output = v4l2_subdev_state_get_format(state, UDS_PAD_SOURCE);
hscale = output->width / input->width;
/*
@@ -356,18 +355,17 @@ static unsigned int uds_max_width(struct vsp1_entity *entity,
*/
static void uds_partition(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int partition_idx,
struct v4l2_rect *window)
{
- struct vsp1_uds *uds = to_uds(&entity->subdev);
const struct v4l2_mbus_framefmt *output;
const struct v4l2_mbus_framefmt *input;
- input = v4l2_subdev_state_get_format(uds->entity.state, UDS_PAD_SINK);
- output = v4l2_subdev_state_get_format(uds->entity.state,
- UDS_PAD_SOURCE);
+ input = v4l2_subdev_state_get_format(state, UDS_PAD_SINK);
+ output = v4l2_subdev_state_get_format(state, UDS_PAD_SOURCE);
partition->uds_sink.width = window->width * input->width
/ output->width;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uif.c b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
index cecd2f7024f4..edaf28b544d2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
@@ -188,6 +188,7 @@ static const struct v4l2_subdev_ops uif_ops = {
*/
static void uif_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -200,7 +201,7 @@ static void uif_configure_stream(struct vsp1_entity *entity,
vsp1_uif_write(uif, dlb, VI6_UIF_DISCOM_DOCMPMR,
VI6_UIF_DISCOM_DOCMPMR_SEL(9));
- crop = v4l2_subdev_state_get_crop(entity->state, UIF_PAD_SINK);
+ crop = v4l2_subdev_state_get_crop(state, UIF_PAD_SINK);
left = crop->left;
width = crop->width;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 10a0485abc6c..c3dd246e707b 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -699,7 +699,9 @@ static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
if (!entity->ops->max_width)
continue;
- entity_max = entity->ops->max_width(entity, pipe);
+ entity_max = entity->ops->max_width(entity,
+ entity->state,
+ pipe);
if (entity_max)
div_size = min(div_size, entity_max);
}
@@ -760,7 +762,7 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
list_for_each_entry(entity, &pipe->entities, list_pipe) {
vsp1_entity_route_setup(entity, pipe, pipe->stream_config);
- vsp1_entity_configure_stream(entity, pipe, NULL,
+ vsp1_entity_configure_stream(entity, entity->state, pipe, NULL,
pipe->stream_config);
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index 5c363ff1d36c..f176750ccd98 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -229,6 +229,7 @@ static int wpf_configure_writeback_chain(struct vsp1_rwpf *wpf,
}
static void wpf_configure_stream(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb)
@@ -243,10 +244,8 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
u32 srcrpf = 0;
int ret;
- sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
- RWPF_PAD_SINK);
- source_format = v4l2_subdev_state_get_format(wpf->entity.state,
- RWPF_PAD_SOURCE);
+ sink_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
+ source_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SOURCE);
/* Format */
if (!pipe->lif || wpf->writeback) {
@@ -496,6 +495,7 @@ static void wpf_configure_partition(struct vsp1_entity *entity,
}
static unsigned int wpf_max_width(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe)
{
struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
@@ -504,6 +504,7 @@ static unsigned int wpf_max_width(struct vsp1_entity *entity,
}
static void wpf_partition(struct vsp1_entity *entity,
+ struct v4l2_subdev_state *state,
struct vsp1_pipeline *pipe,
struct vsp1_partition *partition,
unsigned int partition_idx,
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 17/19] media: renesas: vsp1: Initialize control handler after subdev
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (15 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 16/19] media: renesas: vsp1: Pass subdev state " Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 18/19] media: renesas: vsp1: Switch to V4L2 subdev active state Laurent Pinchart
` (2 subsequent siblings)
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
Some VSP modules initialize their control handler after initializing the
subdev, while some initialize it before. This makes the code
inconsistent and more error prone. Standardize on control initialization
after initializing the subdev.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_hgo.c | 20 +++++++++----------
.../media/platform/renesas/vsp1/vsp1_hgt.c | 12 +++++------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
index 0ea87014a701..2c8ce7175a4e 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
@@ -192,6 +192,16 @@ struct vsp1_hgo *vsp1_hgo_create(struct vsp1_device *vsp1)
if (hgo == NULL)
return ERR_PTR(-ENOMEM);
+ /* Initialize the video device and queue for statistics data. */
+ ret = vsp1_histogram_init(vsp1, &hgo->histo, VSP1_ENTITY_HGO, "hgo",
+ &hgo_entity_ops, hgo_mbus_formats,
+ ARRAY_SIZE(hgo_mbus_formats),
+ HGO_DATA_SIZE, V4L2_META_FMT_VSP1_HGO);
+ if (ret < 0) {
+ vsp1_entity_destroy(&hgo->histo.entity);
+ return ERR_PTR(ret);
+ }
+
/* Initialize the control handler. */
v4l2_ctrl_handler_init(&hgo->ctrls.handler,
vsp1->info->gen >= 3 ? 2 : 1);
@@ -207,15 +217,5 @@ struct vsp1_hgo *vsp1_hgo_create(struct vsp1_device *vsp1)
hgo->histo.entity.subdev.ctrl_handler = &hgo->ctrls.handler;
- /* Initialize the video device and queue for statistics data. */
- ret = vsp1_histogram_init(vsp1, &hgo->histo, VSP1_ENTITY_HGO, "hgo",
- &hgo_entity_ops, hgo_mbus_formats,
- ARRAY_SIZE(hgo_mbus_formats),
- HGO_DATA_SIZE, V4L2_META_FMT_VSP1_HGO);
- if (ret < 0) {
- vsp1_entity_destroy(&hgo->histo.entity);
- return ERR_PTR(ret);
- }
-
return hgo;
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
index 496354c8df0e..858f330d44fa 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
@@ -191,12 +191,6 @@ struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1)
if (hgt == NULL)
return ERR_PTR(-ENOMEM);
- /* Initialize the control handler. */
- v4l2_ctrl_handler_init(&hgt->ctrls, 1);
- v4l2_ctrl_new_custom(&hgt->ctrls, &hgt_hue_areas, NULL);
-
- hgt->histo.entity.subdev.ctrl_handler = &hgt->ctrls;
-
/* Initialize the video device and queue for statistics data. */
ret = vsp1_histogram_init(vsp1, &hgt->histo, VSP1_ENTITY_HGT, "hgt",
&hgt_entity_ops, hgt_mbus_formats,
@@ -207,6 +201,12 @@ struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1)
return ERR_PTR(ret);
}
+ /* Initialize the control handler. */
+ v4l2_ctrl_handler_init(&hgt->ctrls, 1);
+ v4l2_ctrl_new_custom(&hgt->ctrls, &hgt_hue_areas, NULL);
+
+ hgt->histo.entity.subdev.ctrl_handler = &hgt->ctrls;
+
v4l2_ctrl_handler_setup(&hgt->ctrls);
return hgt;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 18/19] media: renesas: vsp1: Switch to V4L2 subdev active state
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (16 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 17/19] media: renesas: vsp1: Initialize control handler after subdev Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2024-06-19 0:17 ` [PATCH v2 19/19] media: renesas: vsp1: Rename all v4l2_subdev_state variables to 'state' Laurent Pinchart
2025-06-26 8:30 ` [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Tomi Valkeinen
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
Replace the subdev state stored in the vsp1_entity structure with the
active state managed by the V4L2 subdev core. This simplifies both code
for individual subdevs and core VSP1 code, as shown by the diffstat. The
only piece of code whose complexity increases is the display pipeline
management, in vsp1_du_pipeline_configure(), which has to lock and
unlock states manually.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_brx.c | 46 ++----
.../media/platform/renesas/vsp1/vsp1_clu.c | 4 +-
.../media/platform/renesas/vsp1/vsp1_drm.c | 31 +++-
.../media/platform/renesas/vsp1/vsp1_entity.c | 134 ++----------------
.../media/platform/renesas/vsp1/vsp1_entity.h | 18 +--
.../media/platform/renesas/vsp1/vsp1_hgo.c | 2 +
.../media/platform/renesas/vsp1/vsp1_hgt.c | 2 +
.../media/platform/renesas/vsp1/vsp1_histo.c | 46 ++----
.../media/platform/renesas/vsp1/vsp1_hsit.c | 20 +--
.../media/platform/renesas/vsp1/vsp1_lif.c | 2 +-
.../media/platform/renesas/vsp1/vsp1_lut.c | 4 +-
.../media/platform/renesas/vsp1/vsp1_pipe.c | 16 ++-
.../media/platform/renesas/vsp1/vsp1_rpf.c | 6 +-
.../media/platform/renesas/vsp1/vsp1_rwpf.c | 57 ++------
.../media/platform/renesas/vsp1/vsp1_sru.c | 41 ++----
.../media/platform/renesas/vsp1/vsp1_uds.c | 39 +----
.../media/platform/renesas/vsp1/vsp1_uif.c | 39 +----
.../media/platform/renesas/vsp1/vsp1_video.c | 15 +-
.../media/platform/renesas/vsp1/vsp1_wpf.c | 11 +-
19 files changed, 136 insertions(+), 397 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index 5dee0490c593..849012b178f4 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -125,21 +125,11 @@ static void brx_try_format(struct vsp1_brx *brx,
}
static int brx_set_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_format *fmt)
{
struct vsp1_brx *brx = to_brx(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
- int ret = 0;
-
- mutex_lock(&brx->entity.lock);
-
- state = vsp1_entity_get_state(&brx->entity, sd_state, fmt->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
brx_try_format(brx, state, fmt->pad, &fmt->format);
@@ -167,17 +157,14 @@ static int brx_set_format(struct v4l2_subdev *subdev,
}
}
-done:
- mutex_unlock(&brx->entity.lock);
- return ret;
+ return 0;
}
static int brx_get_selection(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
struct vsp1_brx *brx = to_brx(subdev);
- struct v4l2_subdev_state *state;
if (sel->pad == brx->entity.source_pad)
return -EINVAL;
@@ -191,14 +178,7 @@ static int brx_get_selection(struct v4l2_subdev *subdev,
return 0;
case V4L2_SEL_TGT_COMPOSE:
- state = vsp1_entity_get_state(&brx->entity, sd_state,
- sel->which);
- if (!state)
- return -EINVAL;
-
- mutex_lock(&brx->entity.lock);
sel->r = *v4l2_subdev_state_get_compose(state, sel->pad);
- mutex_unlock(&brx->entity.lock);
return 0;
default:
@@ -207,14 +187,12 @@ static int brx_get_selection(struct v4l2_subdev *subdev,
}
static int brx_set_selection(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
struct vsp1_brx *brx = to_brx(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *compose;
- int ret = 0;
if (sel->pad == brx->entity.source_pad)
return -EINVAL;
@@ -222,14 +200,6 @@ static int brx_set_selection(struct v4l2_subdev *subdev,
if (sel->target != V4L2_SEL_TGT_COMPOSE)
return -EINVAL;
- mutex_lock(&brx->entity.lock);
-
- state = vsp1_entity_get_state(&brx->entity, sd_state, sel->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
-
/*
* The compose rectangle top left corner must be inside the output
* frame.
@@ -249,15 +219,13 @@ static int brx_set_selection(struct v4l2_subdev *subdev,
compose = v4l2_subdev_state_get_compose(state, sel->pad);
*compose = sel->r;
-done:
- mutex_unlock(&brx->entity.lock);
- return ret;
+ return 0;
}
static const struct v4l2_subdev_pad_ops brx_pad_ops = {
.enum_mbus_code = brx_enum_mbus_code,
.enum_frame_size = brx_enum_frame_size,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = brx_set_format,
.get_selection = brx_get_selection,
.set_selection = brx_set_selection,
@@ -425,6 +393,8 @@ struct vsp1_brx *vsp1_brx_create(struct vsp1_device *vsp1,
/* Initialize the control handler. */
v4l2_ctrl_handler_init(&brx->ctrls, 1);
+ brx->ctrls.lock = &brx->entity.subdev.active_state->_lock;
+
v4l2_ctrl_new_std(&brx->ctrls, &brx_ctrl_ops, V4L2_CID_BG_COLOR,
0, 0xffffff, 1, 0);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_clu.c b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
index 98645bd2a983..d1340e3f3e69 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
@@ -157,7 +157,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_frame_size = clu_enum_frame_size,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = clu_set_format,
};
@@ -266,6 +266,8 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device *vsp1)
/* Initialize the control handler. */
v4l2_ctrl_handler_init(&clu->ctrls, 2);
+ clu->ctrls.lock = &clu->entity.subdev.active_state->_lock;
+
v4l2_ctrl_new_custom(&clu->ctrls, &clu_table_control, NULL);
v4l2_ctrl_new_custom(&clu->ctrls, &clu_mode_control, NULL);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index b5d1f238f7be..0884312da752 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -550,9 +550,6 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
struct vsp1_dl_body *dlb;
unsigned int dl_flags = 0;
- vsp1_pipeline_calculate_partition(pipe, &pipe->part_table[0],
- drm_pipe->width, 0);
-
if (drm_pipe->force_brx_release)
dl_flags |= VSP1_DL_FRAME_END_INTERNAL;
if (pipe->output->writeback)
@@ -561,8 +558,11 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
dl = vsp1_dl_list_get(pipe->output->dlm);
dlb = vsp1_dl_list_get_body0(dl);
+ /*
+ * Lock the state for all entities in the pipeline, disconnecting
+ * unused entities along the way.
+ */
list_for_each_entry_safe(entity, next, &pipe->entities, list_pipe) {
- /* Disconnect unused entities from the pipeline. */
if (!entity->pipe) {
vsp1_dl_body_write(dlb, entity->route->reg,
VI6_DPR_NODE_UNUSED);
@@ -573,14 +573,33 @@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
continue;
}
+ v4l2_subdev_get_locked_active_state(&entity->subdev);
+ }
+
+ vsp1_pipeline_calculate_partition(pipe, &pipe->part_table[0],
+ drm_pipe->width, 0);
+
+ /* Configure the pipeline. */
+ list_for_each_entry(entity, &pipe->entities, list_pipe) {
+ struct v4l2_subdev_state *state;
+
+ state = v4l2_subdev_get_locked_active_state(&entity->subdev);
+
vsp1_entity_route_setup(entity, pipe, dlb);
- vsp1_entity_configure_stream(entity, entity->state, pipe,
- dl, dlb);
+ vsp1_entity_configure_stream(entity, state, pipe, dl, dlb);
vsp1_entity_configure_frame(entity, pipe, dl, dlb);
vsp1_entity_configure_partition(entity, pipe,
&pipe->part_table[0], dl, dlb);
}
+ /* Unlock all states. */
+ list_for_each_entry_reverse(entity, &pipe->entities, list_pipe) {
+ struct v4l2_subdev_state *state;
+
+ state = v4l2_subdev_get_unlocked_active_state(&entity->subdev);
+ v4l2_subdev_unlock_state(state);
+ }
+
vsp1_dl_list_commit(dl, dl_flags);
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 8b8945bd8f10..f4e6e2f27182 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -103,64 +103,10 @@ void vsp1_entity_configure_partition(struct vsp1_entity *entity,
* V4L2 Subdevice Operations
*/
-/**
- * vsp1_entity_get_state - Get the subdev state for an entity
- * @entity: the entity
- * @sd_state: the TRY state
- * @which: state selector (ACTIVE or TRY)
- *
- * When called with which set to V4L2_SUBDEV_FORMAT_ACTIVE the caller must hold
- * the entity lock to access the returned configuration.
- *
- * Return the subdev state requested by the which argument. The TRY state is
- * passed explicitly to the function through the sd_state argument and simply
- * returned when requested. The ACTIVE state comes from the entity structure.
- */
-struct v4l2_subdev_state *
-vsp1_entity_get_state(struct vsp1_entity *entity,
- struct v4l2_subdev_state *sd_state,
- enum v4l2_subdev_format_whence which)
-{
- switch (which) {
- case V4L2_SUBDEV_FORMAT_ACTIVE:
- return entity->state;
- case V4L2_SUBDEV_FORMAT_TRY:
- default:
- return sd_state;
- }
-}
-
-/*
- * vsp1_subdev_get_pad_format - Subdev pad get_fmt handler
- * @subdev: V4L2 subdevice
- * @sd_state: V4L2 subdev state
- * @fmt: V4L2 subdev format
- *
- * This function implements the subdev get_fmt pad operation. It can be used as
- * a direct drop-in for the operation handler.
- */
-int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt)
-{
- struct vsp1_entity *entity = to_vsp1_entity(subdev);
- struct v4l2_subdev_state *state;
-
- state = vsp1_entity_get_state(entity, sd_state, fmt->which);
- if (!state)
- return -EINVAL;
-
- mutex_lock(&entity->lock);
- fmt->format = *v4l2_subdev_state_get_format(state, fmt->pad);
- mutex_unlock(&entity->lock);
-
- return 0;
-}
-
/*
* vsp1_subdev_enum_mbus_code - Subdev pad enum_mbus_code handler
* @subdev: V4L2 subdevice
- * @sd_state: V4L2 subdev state
+ * @state: V4L2 subdev state
* @code: Media bus code enumeration
* @codes: Array of supported media bus codes
* @ncodes: Number of supported media bus codes
@@ -171,19 +117,16 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
* the sink pad.
*/
int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code,
const unsigned int *codes, unsigned int ncodes)
{
- struct vsp1_entity *entity = to_vsp1_entity(subdev);
-
if (code->pad == 0) {
if (code->index >= ncodes)
return -EINVAL;
code->code = codes[code->index];
} else {
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
/*
@@ -193,14 +136,8 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
if (code->index)
return -EINVAL;
- state = vsp1_entity_get_state(entity, sd_state, code->which);
- if (!state)
- return -EINVAL;
-
- mutex_lock(&entity->lock);
format = v4l2_subdev_state_get_format(state, 0);
code->code = format->code;
- mutex_unlock(&entity->lock);
}
return 0;
@@ -209,7 +146,7 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
/*
* vsp1_subdev_enum_frame_size - Subdev pad enum_frame_size handler
* @subdev: V4L2 subdevice
- * @sd_state: V4L2 subdev state
+ * @state: V4L2 subdev state
* @fse: Frame size enumeration
* @min_width: Minimum image width
* @min_height: Minimum image height
@@ -222,28 +159,17 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
* source pad size identical to the sink pad.
*/
int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *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 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)
- 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->index || fse->code != format->code)
+ return -EINVAL;
if (fse->pad == 0) {
fse->min_width = min_width;
@@ -261,15 +187,13 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
fse->max_height = format->height;
}
-done:
- mutex_unlock(&entity->lock);
- return ret;
+ return 0;
}
/*
* vsp1_subdev_set_pad_format - Subdev pad set_fmt handler
* @subdev: V4L2 subdevice
- * @sd_state: V4L2 subdev state
+ * @state: V4L2 subdev state
* @fmt: V4L2 subdev format
* @codes: Array of supported media bus codes
* @ncodes: Number of supported media bus codes
@@ -285,33 +209,23 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
* source pad.
*/
int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *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)
{
struct vsp1_entity *entity = to_vsp1_entity(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *selection;
unsigned int i;
- int ret = 0;
-
- mutex_lock(&entity->lock);
-
- state = vsp1_entity_get_state(entity, sd_state, fmt->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
format = v4l2_subdev_state_get_format(state, fmt->pad);
if (fmt->pad == entity->source_pad) {
/* The output format can't be modified. */
fmt->format = *format;
- goto done;
+ return 0;
}
/*
@@ -350,9 +264,7 @@ int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
selection->width = format->width;
selection->height = format->height;
-done:
- mutex_unlock(&entity->lock);
- return ret;
+ return 0;
}
static int vsp1_entity_init_state(struct v4l2_subdev *subdev,
@@ -364,8 +276,6 @@ static int vsp1_entity_init_state(struct v4l2_subdev *subdev,
for (pad = 0; pad < subdev->entity.num_pads - 1; ++pad) {
struct v4l2_subdev_format format = {
.pad = pad,
- .which = sd_state ? V4L2_SUBDEV_FORMAT_TRY
- : V4L2_SUBDEV_FORMAT_ACTIVE,
};
v4l2_subdev_call(subdev, pad, set_fmt, sd_state, &format);
@@ -563,7 +473,6 @@ 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)
{
- static struct lock_class_key key;
struct v4l2_subdev *subdev;
unsigned int i;
int ret;
@@ -579,8 +488,6 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,
if (i == ARRAY_SIZE(vsp1_routes))
return -EINVAL;
- mutex_init(&entity->lock);
-
entity->vsp1 = vsp1;
entity->source_pad = num_pads - 1;
@@ -621,21 +528,10 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,
snprintf(subdev->name, sizeof(subdev->name), "%s %s",
dev_name(vsp1->dev), name);
- vsp1_entity_init_state(subdev, NULL);
-
- /*
- * Allocate the subdev state to store formats and selection
- * rectangles.
- */
- /*
- * FIXME: Drop this call, drivers are not supposed to use
- * __v4l2_subdev_state_alloc().
- */
- entity->state = __v4l2_subdev_state_alloc(&entity->subdev,
- "vsp1:state->lock", &key);
- if (IS_ERR(entity->state)) {
+ ret = v4l2_subdev_init_finalize(subdev);
+ if (ret) {
media_entity_cleanup(&entity->subdev.entity);
- return PTR_ERR(entity->state);
+ return ret;
}
return 0;
@@ -647,6 +543,6 @@ void vsp1_entity_destroy(struct vsp1_entity *entity)
entity->ops->destroy(entity);
if (entity->subdev.ctrl_handler)
v4l2_ctrl_handler_free(entity->subdev.ctrl_handler);
- __v4l2_subdev_state_free(entity->state);
+ v4l2_subdev_cleanup(&entity->subdev);
media_entity_cleanup(&entity->subdev.entity);
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index 1bcc9e27dfdc..411d264448d9 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -10,7 +10,6 @@
#define __VSP1_ENTITY_H__
#include <linux/list.h>
-#include <linux/mutex.h>
#include <media/v4l2-subdev.h>
@@ -125,9 +124,6 @@ struct vsp1_entity {
unsigned int sink_pad;
struct v4l2_subdev subdev;
- struct v4l2_subdev_state *state;
-
- struct mutex lock; /* Protects the state */
};
static inline struct vsp1_entity *to_vsp1_entity(struct v4l2_subdev *subdev)
@@ -144,11 +140,6 @@ int vsp1_entity_link_setup(struct media_entity *entity,
const struct media_pad *local,
const struct media_pad *remote, u32 flags);
-struct v4l2_subdev_state *
-vsp1_entity_get_state(struct vsp1_entity *entity,
- struct v4l2_subdev_state *sd_state,
- enum v4l2_subdev_format_whence which);
-
void vsp1_entity_route_setup(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
struct vsp1_dl_body *dlb);
@@ -172,21 +163,18 @@ void vsp1_entity_configure_partition(struct vsp1_entity *entity,
struct media_pad *vsp1_entity_remote_pad(struct media_pad *pad);
-int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *fmt);
int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *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_state *state,
struct v4l2_subdev_mbus_code_enum *code,
const unsigned int *codes, unsigned int ncodes);
int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_frame_size_enum *fse,
unsigned int min_w, unsigned int min_h,
unsigned int max_w, unsigned int max_h);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
index 2c8ce7175a4e..561c86e889a9 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
@@ -205,6 +205,8 @@ struct vsp1_hgo *vsp1_hgo_create(struct vsp1_device *vsp1)
/* Initialize the control handler. */
v4l2_ctrl_handler_init(&hgo->ctrls.handler,
vsp1->info->gen >= 3 ? 2 : 1);
+ hgo->ctrls.handler.lock = &hgo->histo.entity.subdev.active_state->_lock;
+
hgo->ctrls.max_rgb = v4l2_ctrl_new_custom(&hgo->ctrls.handler,
&hgo_max_rgb_control, NULL);
if (vsp1->info->gen >= 3)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
index 858f330d44fa..d26433fe54c6 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
@@ -203,6 +203,8 @@ struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1)
/* Initialize the control handler. */
v4l2_ctrl_handler_init(&hgt->ctrls, 1);
+ hgt->ctrls.lock = &hgt->histo.entity.subdev.active_state->_lock;
+
v4l2_ctrl_new_custom(&hgt->ctrls, &hgt_hue_areas, NULL);
hgt->histo.entity.subdev.ctrl_handler = &hgt->ctrls;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_histo.c b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
index 9c2d4c91bfad..d3bf7a281646 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
@@ -195,26 +195,15 @@ static int histo_enum_frame_size(struct v4l2_subdev *subdev,
}
static int histo_get_selection(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
- struct vsp1_histogram *histo = subdev_to_histo(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *crop;
- int ret = 0;
if (sel->pad != HISTO_PAD_SINK)
return -EINVAL;
- mutex_lock(&histo->entity.lock);
-
- state = vsp1_entity_get_state(&histo->entity, sd_state, sel->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
-
switch (sel->target) {
case V4L2_SEL_TGT_COMPOSE_BOUNDS:
case V4L2_SEL_TGT_COMPOSE_DEFAULT:
@@ -243,13 +232,10 @@ static int histo_get_selection(struct v4l2_subdev *subdev,
break;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
-done:
- mutex_unlock(&histo->entity.lock);
- return ret;
+ return 0;
}
static int histo_set_crop(struct v4l2_subdev *subdev,
@@ -322,34 +308,18 @@ static int histo_set_compose(struct v4l2_subdev *subdev,
}
static int histo_set_selection(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
- struct vsp1_histogram *histo = subdev_to_histo(subdev);
- struct v4l2_subdev_state *state;
- int ret;
-
if (sel->pad != HISTO_PAD_SINK)
return -EINVAL;
- mutex_lock(&histo->entity.lock);
-
- state = vsp1_entity_get_state(&histo->entity, sd_state, sel->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
-
if (sel->target == V4L2_SEL_TGT_CROP)
- ret = histo_set_crop(subdev, state, sel);
+ return histo_set_crop(subdev, state, sel);
else if (sel->target == V4L2_SEL_TGT_COMPOSE)
- ret = histo_set_compose(subdev, state, sel);
+ return histo_set_compose(subdev, state, sel);
else
- ret = -EINVAL;
-
-done:
- mutex_unlock(&histo->entity.lock);
- return ret;
+ return -EINVAL;
}
static int histo_set_format(struct v4l2_subdev *subdev,
@@ -377,7 +347,7 @@ static int histo_set_format(struct v4l2_subdev *subdev,
static const struct v4l2_subdev_pad_ops histo_pad_ops = {
.enum_mbus_code = histo_enum_mbus_code,
.enum_frame_size = histo_enum_frame_size,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = histo_set_format,
.get_selection = histo_get_selection,
.set_selection = histo_set_selection,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
index 8ba2a7c7305c..069d3de1797f 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
@@ -62,21 +62,11 @@ static int hsit_enum_frame_size(struct v4l2_subdev *subdev,
}
static int hsit_set_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_format *fmt)
{
struct vsp1_hsit *hsit = to_hsit(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
- int ret = 0;
-
- mutex_lock(&hsit->entity.lock);
-
- state = vsp1_entity_get_state(&hsit->entity, sd_state, fmt->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
format = v4l2_subdev_state_get_format(state, fmt->pad);
@@ -86,7 +76,7 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
* modified.
*/
fmt->format = *format;
- goto done;
+ return 0;
}
format->code = hsit->inverse ? MEDIA_BUS_FMT_AHSV8888_1X32
@@ -106,15 +96,13 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
format->code = hsit->inverse ? MEDIA_BUS_FMT_ARGB8888_1X32
: MEDIA_BUS_FMT_AHSV8888_1X32;
-done:
- mutex_unlock(&hsit->entity.lock);
- return ret;
+ return 0;
}
static const struct v4l2_subdev_pad_ops hsit_pad_ops = {
.enum_mbus_code = hsit_enum_mbus_code,
.enum_frame_size = hsit_enum_frame_size,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = hsit_set_format,
};
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
index b3d83d1c5306..f1fefa2817d4 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
@@ -70,7 +70,7 @@ static int lif_set_format(struct v4l2_subdev *subdev,
static const struct v4l2_subdev_pad_ops lif_pad_ops = {
.enum_mbus_code = lif_enum_mbus_code,
.enum_frame_size = lif_enum_frame_size,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = lif_set_format,
};
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lut.c b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
index dd264e6532e0..1063076ad33f 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
@@ -133,7 +133,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_frame_size = lut_enum_frame_size,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = lut_set_format,
};
@@ -225,6 +225,8 @@ struct vsp1_lut *vsp1_lut_create(struct vsp1_device *vsp1)
/* Initialize the control handler. */
v4l2_ctrl_handler_init(&lut->ctrls, 1);
+ lut->ctrls.lock = &lut->entity.subdev.active_state->_lock;
+
v4l2_ctrl_new_custom(&lut->ctrls, &lut_table_control, NULL);
lut->entity.subdev.ctrl_handler = &lut->ctrls;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
index b392115595f2..5c5b9835c74b 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
@@ -486,9 +486,14 @@ static void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe,
struct vsp1_entity *entity;
list_for_each_entry_reverse(entity, &pipe->entities, list_pipe) {
- if (entity->ops->partition)
- entity->ops->partition(entity, entity->state, pipe,
- partition, index, window);
+ struct v4l2_subdev_state *state;
+
+ if (!entity->ops->partition)
+ continue;
+
+ state = v4l2_subdev_get_locked_active_state(&entity->subdev);
+ entity->ops->partition(entity, state, pipe, partition, index,
+ window);
}
}
@@ -507,6 +512,7 @@ void vsp1_pipeline_calculate_partition(struct vsp1_pipeline *pipe,
unsigned int index)
{
const struct v4l2_mbus_framefmt *format;
+ struct v4l2_subdev_state *state;
struct v4l2_rect window;
unsigned int modulus;
@@ -514,8 +520,8 @@ void vsp1_pipeline_calculate_partition(struct vsp1_pipeline *pipe,
* Partitions are computed on the size before rotation, use the format
* at the WPF sink.
*/
- format = v4l2_subdev_state_get_format(pipe->output->entity.state,
- RWPF_PAD_SINK);
+ state = v4l2_subdev_get_locked_active_state(&pipe->output->entity.subdev);
+ format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
/* Initialise the partition with sane starting conditions. */
window.left = index * div_size;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
index 5c8b3ba1bd3c..66a6f2eb2ae3 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
@@ -152,9 +152,13 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
/* Output location. */
if (pipe->brx) {
+ struct v4l2_subdev *brx_subdev = &pipe->brx->subdev;
+ struct v4l2_subdev_state *brx_state;
const struct v4l2_rect *compose;
- compose = v4l2_subdev_state_get_compose(pipe->brx->state,
+ brx_state = v4l2_subdev_get_locked_active_state(brx_subdev);
+
+ compose = v4l2_subdev_state_get_compose(brx_state,
rpf->brx_input);
left = compose->left;
top = compose->top;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index 9d38203e73d0..e4edce3bf584 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -51,21 +51,11 @@ static int vsp1_rwpf_enum_frame_size(struct v4l2_subdev *subdev,
}
static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_format *fmt)
{
struct vsp1_rwpf *rwpf = to_rwpf(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
- int ret = 0;
-
- mutex_lock(&rwpf->entity.lock);
-
- state = vsp1_entity_get_state(&rwpf->entity, sd_state, fmt->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
/* Default to YUV if the requested format is not supported. */
if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
@@ -82,7 +72,7 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
*/
format->code = fmt->format.code;
fmt->format = *format;
- goto done;
+ return 0;
}
format->code = fmt->format.code;
@@ -115,19 +105,15 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
format->height = fmt->format.width;
}
-done:
- mutex_unlock(&rwpf->entity.lock);
- return ret;
+ return 0;
}
static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
struct vsp1_rwpf *rwpf = to_rwpf(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
- int ret = 0;
/*
* Cropping is only supported on the RPF and is implemented on the sink
@@ -136,14 +122,6 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
if (rwpf->entity.type == VSP1_ENTITY_WPF || sel->pad != RWPF_PAD_SINK)
return -EINVAL;
- mutex_lock(&rwpf->entity.lock);
-
- state = vsp1_entity_get_state(&rwpf->entity, sd_state, sel->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
-
switch (sel->target) {
case V4L2_SEL_TGT_CROP:
sel->r = *v4l2_subdev_state_get_crop(state, RWPF_PAD_SINK);
@@ -158,24 +136,19 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev *subdev,
break;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
-done:
- mutex_unlock(&rwpf->entity.lock);
- return ret;
+ return 0;
}
static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
struct vsp1_rwpf *rwpf = to_rwpf(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *crop;
- int ret = 0;
/*
* Cropping is only supported on the RPF and is implemented on the sink
@@ -187,14 +160,6 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
if (sel->target != V4L2_SEL_TGT_CROP)
return -EINVAL;
- mutex_lock(&rwpf->entity.lock);
-
- state = vsp1_entity_get_state(&rwpf->entity, sd_state, sel->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
-
/* Make sure the crop rectangle is entirely contained in the image. */
format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
@@ -224,15 +189,13 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
format->width = crop->width;
format->height = crop->height;
-done:
- mutex_unlock(&rwpf->entity.lock);
- return ret;
+ return 0;
}
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,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = vsp1_rwpf_set_format,
.get_selection = vsp1_rwpf_get_selection,
.set_selection = vsp1_rwpf_set_selection,
@@ -267,6 +230,8 @@ static const struct v4l2_ctrl_ops vsp1_rwpf_ctrl_ops = {
int vsp1_rwpf_init_ctrls(struct vsp1_rwpf *rwpf, unsigned int ncontrols)
{
v4l2_ctrl_handler_init(&rwpf->ctrls, ncontrols + 1);
+ rwpf->ctrls.lock = &rwpf->entity.subdev.active_state->_lock;
+
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 1759ce642e6e..a5c839381b84 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -119,26 +119,15 @@ static int sru_enum_mbus_code(struct v4l2_subdev *subdev,
}
static int sru_enum_frame_size(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
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;
-
- state = vsp1_entity_get_state(&sru->entity, sd_state, fse->which);
- if (!state)
- return -EINVAL;
format = v4l2_subdev_state_get_format(state, SRU_PAD_SINK);
- mutex_lock(&sru->entity.lock);
-
- if (fse->index || fse->code != format->code) {
- ret = -EINVAL;
- goto done;
- }
+ if (fse->index || fse->code != format->code)
+ return -EINVAL;
if (fse->pad == SRU_PAD_SINK) {
fse->min_width = SRU_MIN_SIZE;
@@ -158,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,
@@ -215,21 +202,11 @@ static void sru_try_format(struct vsp1_sru *sru,
}
static int sru_set_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_format *fmt)
{
struct vsp1_sru *sru = to_sru(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
- int ret = 0;
-
- mutex_lock(&sru->entity.lock);
-
- state = vsp1_entity_get_state(&sru->entity, sd_state, fmt->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
sru_try_format(sru, state, fmt->pad, &fmt->format);
@@ -244,15 +221,13 @@ static int sru_set_format(struct v4l2_subdev *subdev,
sru_try_format(sru, state, SRU_PAD_SOURCE, format);
}
-done:
- mutex_unlock(&sru->entity.lock);
- return ret;
+ return 0;
}
static const struct v4l2_subdev_pad_ops sru_pad_ops = {
.enum_mbus_code = sru_enum_mbus_code,
.enum_frame_size = sru_enum_frame_size,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = sru_set_format,
};
@@ -371,6 +346,8 @@ struct vsp1_sru *vsp1_sru_create(struct vsp1_device *vsp1)
/* Initialize the control handler. */
v4l2_ctrl_handler_init(&sru->ctrls, 1);
+ sru->ctrls.lock = &sru->entity.subdev.active_state->_lock;
+
v4l2_ctrl_new_custom(&sru->ctrls, &sru_intensity_control, NULL);
sru->intensity = 1;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index c5a38478cf8c..fd28504d3538 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -124,26 +124,15 @@ static int uds_enum_mbus_code(struct v4l2_subdev *subdev,
}
static int uds_enum_frame_size(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
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;
-
- state = vsp1_entity_get_state(&uds->entity, sd_state, fse->which);
- if (!state)
- return -EINVAL;
format = v4l2_subdev_state_get_format(state, UDS_PAD_SINK);
- mutex_lock(&uds->entity.lock);
-
- if (fse->index || fse->code != format->code) {
- ret = -EINVAL;
- goto done;
- }
+ if (fse->index || fse->code != format->code)
+ return -EINVAL;
if (fse->pad == UDS_PAD_SINK) {
fse->min_width = UDS_MIN_SIZE;
@@ -157,9 +146,7 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
&fse->max_height);
}
-done:
- mutex_unlock(&uds->entity.lock);
- return ret;
+ return 0;
}
static void uds_try_format(struct vsp1_uds *uds,
@@ -198,21 +185,11 @@ static void uds_try_format(struct vsp1_uds *uds,
}
static int uds_set_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_format *fmt)
{
struct vsp1_uds *uds = to_uds(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
- int ret = 0;
-
- mutex_lock(&uds->entity.lock);
-
- state = vsp1_entity_get_state(&uds->entity, sd_state, fmt->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
uds_try_format(uds, state, fmt->pad, &fmt->format);
@@ -227,9 +204,7 @@ static int uds_set_format(struct v4l2_subdev *subdev,
uds_try_format(uds, state, UDS_PAD_SOURCE, format);
}
-done:
- mutex_unlock(&uds->entity.lock);
- return ret;
+ return 0;
}
/* -----------------------------------------------------------------------------
@@ -239,7 +214,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_frame_size = uds_enum_frame_size,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = uds_set_format,
};
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uif.c b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
index edaf28b544d2..911543ee702b 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
@@ -82,25 +82,14 @@ static int uif_set_format(struct v4l2_subdev *subdev,
}
static int uif_get_selection(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
- struct vsp1_uif *uif = to_uif(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
- int ret = 0;
if (sel->pad != UIF_PAD_SINK)
return -EINVAL;
- mutex_lock(&uif->entity.lock);
-
- state = vsp1_entity_get_state(&uif->entity, sd_state, sel->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
-
switch (sel->target) {
case V4L2_SEL_TGT_CROP_BOUNDS:
case V4L2_SEL_TGT_CROP_DEFAULT:
@@ -116,37 +105,23 @@ static int uif_get_selection(struct v4l2_subdev *subdev,
break;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
-done:
- mutex_unlock(&uif->entity.lock);
- return ret;
+ return 0;
}
static int uif_set_selection(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
- struct vsp1_uif *uif = to_uif(subdev);
- struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *format;
struct v4l2_rect *selection;
- int ret = 0;
if (sel->pad != UIF_PAD_SINK ||
sel->target != V4L2_SEL_TGT_CROP)
return -EINVAL;
- mutex_lock(&uif->entity.lock);
-
- state = vsp1_entity_get_state(&uif->entity, sd_state, sel->which);
- if (!state) {
- ret = -EINVAL;
- goto done;
- }
-
/* The crop rectangle must be inside the input frame. */
format = v4l2_subdev_state_get_format(state, UIF_PAD_SINK);
@@ -161,9 +136,7 @@ static int uif_set_selection(struct v4l2_subdev *subdev,
selection = v4l2_subdev_state_get_crop(state, sel->pad);
*selection = sel->r;
-done:
- mutex_unlock(&uif->entity.lock);
- return ret;
+ return 0;
}
/* -----------------------------------------------------------------------------
@@ -173,7 +146,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_frame_size = uif_enum_frame_size,
- .get_fmt = vsp1_subdev_get_pad_format,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = uif_set_format,
.get_selection = uif_get_selection,
.set_selection = uif_set_selection,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index c3dd246e707b..4e3f0adb5b08 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -676,6 +676,7 @@ static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
{
struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
const struct v4l2_mbus_framefmt *format;
+ struct v4l2_subdev_state *state;
struct vsp1_entity *entity;
unsigned int div_size;
unsigned int i;
@@ -684,8 +685,8 @@ static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
* Partitions are computed on the size before rotation, use the format
* at the WPF sink.
*/
- format = v4l2_subdev_state_get_format(pipe->output->entity.state,
- RWPF_PAD_SINK);
+ state = v4l2_subdev_get_locked_active_state(&pipe->output->entity.subdev);
+ format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
div_size = format->width;
/*
@@ -699,9 +700,8 @@ static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
if (!entity->ops->max_width)
continue;
- entity_max = entity->ops->max_width(entity,
- entity->state,
- pipe);
+ state = v4l2_subdev_get_locked_active_state(&entity->subdev);
+ entity_max = entity->ops->max_width(entity, state, pipe);
if (entity_max)
div_size = min(div_size, entity_max);
}
@@ -761,8 +761,11 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
return -ENOMEM;
list_for_each_entry(entity, &pipe->entities, list_pipe) {
+ struct v4l2_subdev_state *state;
+
+ state = v4l2_subdev_get_locked_active_state(&entity->subdev);
vsp1_entity_route_setup(entity, pipe, pipe->stream_config);
- vsp1_entity_configure_stream(entity, entity->state, pipe, NULL,
+ vsp1_entity_configure_stream(entity, state, pipe, NULL,
pipe->stream_config);
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
index f176750ccd98..8792b41f307a 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
@@ -44,6 +44,7 @@ enum wpf_flip_ctrl {
static int vsp1_wpf_set_rotation(struct vsp1_rwpf *wpf, unsigned int rotation)
{
struct vsp1_video *video = wpf->video;
+ struct v4l2_subdev_state *state;
struct v4l2_mbus_framefmt *sink_format;
struct v4l2_mbus_framefmt *source_format;
bool rotate;
@@ -65,12 +66,10 @@ static int vsp1_wpf_set_rotation(struct vsp1_rwpf *wpf, unsigned int rotation)
goto done;
}
- sink_format = v4l2_subdev_state_get_format(wpf->entity.state,
- RWPF_PAD_SINK);
- source_format = v4l2_subdev_state_get_format(wpf->entity.state,
- RWPF_PAD_SOURCE);
+ state = v4l2_subdev_get_locked_active_state(&wpf->entity.subdev);
- mutex_lock(&wpf->entity.lock);
+ sink_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
+ source_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SOURCE);
if (rotate) {
source_format->width = sink_format->height;
@@ -82,8 +81,6 @@ static int vsp1_wpf_set_rotation(struct vsp1_rwpf *wpf, unsigned int rotation)
wpf->flip.rotate = rotate;
- mutex_unlock(&wpf->entity.lock);
-
done:
mutex_unlock(&video->lock);
return ret;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 19/19] media: renesas: vsp1: Rename all v4l2_subdev_state variables to 'state'
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (17 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 18/19] media: renesas: vsp1: Switch to V4L2 subdev active state Laurent Pinchart
@ 2024-06-19 0:17 ` Laurent Pinchart
2025-06-26 8:30 ` [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Tomi Valkeinen
19 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2024-06-19 0:17 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Tomi Valkeinen, Jacopo Mondi,
Kieran Bingham
The driver names variables (including function parameters) of
struct *v4l2_subdev_state type either 'state' or 'sd_state'. Rename them
all to 'state' for consistency.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
.../media/platform/renesas/vsp1/vsp1_brx.c | 11 ++++----
.../media/platform/renesas/vsp1/vsp1_clu.c | 12 ++++-----
.../media/platform/renesas/vsp1/vsp1_entity.c | 4 +--
.../media/platform/renesas/vsp1/vsp1_histo.c | 26 +++++++++----------
.../media/platform/renesas/vsp1/vsp1_hsit.c | 6 ++---
.../media/platform/renesas/vsp1/vsp1_lif.c | 12 ++++-----
.../media/platform/renesas/vsp1/vsp1_lut.c | 12 ++++-----
.../media/platform/renesas/vsp1/vsp1_rwpf.c | 6 ++---
.../media/platform/renesas/vsp1/vsp1_sru.c | 8 +++---
.../media/platform/renesas/vsp1/vsp1_uds.c | 8 +++---
.../media/platform/renesas/vsp1/vsp1_uif.c | 12 ++++-----
11 files changed, 58 insertions(+), 59 deletions(-)
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index 849012b178f4..53c97bef44c1 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -65,7 +65,7 @@ static const struct v4l2_ctrl_ops brx_ctrl_ops = {
*/
static int brx_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
static const unsigned int codes[] = {
@@ -73,12 +73,12 @@ static int brx_enum_mbus_code(struct v4l2_subdev *subdev,
MEDIA_BUS_FMT_AYUV8_1X32,
};
- return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, codes,
+ return vsp1_subdev_enum_mbus_code(subdev, 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_state *state,
struct v4l2_subdev_frame_size_enum *fse)
{
if (fse->index)
@@ -97,7 +97,7 @@ static int brx_enum_frame_size(struct v4l2_subdev *subdev,
}
static void brx_try_format(struct vsp1_brx *brx,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
unsigned int pad, struct v4l2_mbus_framefmt *fmt)
{
struct v4l2_mbus_framefmt *format;
@@ -112,8 +112,7 @@ static void brx_try_format(struct vsp1_brx *brx,
default:
/* The BRx can't perform format conversion. */
- format = v4l2_subdev_state_get_format(sd_state,
- BRX_PAD_SINK(0));
+ format = v4l2_subdev_state_get_format(state, BRX_PAD_SINK(0));
fmt->code = format->code;
break;
}
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_clu.c b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
index d1340e3f3e69..bf88db8f4ff2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_clu.c
@@ -123,28 +123,28 @@ static const unsigned int clu_codes[] = {
};
static int clu_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
- return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, clu_codes,
+ return vsp1_subdev_enum_mbus_code(subdev, 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_state *state,
struct v4l2_subdev_frame_size_enum *fse)
{
- return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
+ return vsp1_subdev_enum_frame_size(subdev, 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_state *state,
struct v4l2_subdev_format *fmt)
{
- return vsp1_subdev_set_pad_format(subdev, sd_state, fmt, clu_codes,
+ return vsp1_subdev_set_pad_format(subdev, state, fmt, clu_codes,
ARRAY_SIZE(clu_codes),
CLU_MIN_SIZE, CLU_MIN_SIZE,
CLU_MAX_SIZE, CLU_MAX_SIZE);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index f4e6e2f27182..66308659ffc0 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -268,7 +268,7 @@ int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
}
static int vsp1_entity_init_state(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state)
+ struct v4l2_subdev_state *state)
{
unsigned int pad;
@@ -278,7 +278,7 @@ static int vsp1_entity_init_state(struct v4l2_subdev *subdev,
.pad = pad,
};
- v4l2_subdev_call(subdev, pad, set_fmt, sd_state, &format);
+ v4l2_subdev_call(subdev, pad, set_fmt, state, &format);
}
return 0;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_histo.c b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
index d3bf7a281646..6cebd7055c5e 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_histo.c
@@ -166,7 +166,7 @@ static const struct vb2_ops histo_video_queue_qops = {
*/
static int histo_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
struct vsp1_histogram *histo = subdev_to_histo(subdev);
@@ -176,19 +176,19 @@ static int histo_enum_mbus_code(struct v4l2_subdev *subdev,
return 0;
}
- return vsp1_subdev_enum_mbus_code(subdev, sd_state, code,
+ return vsp1_subdev_enum_mbus_code(subdev, state, code,
histo->formats,
histo->num_formats);
}
static int histo_enum_frame_size(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_frame_size_enum *fse)
{
if (fse->pad != HISTO_PAD_SINK)
return -EINVAL;
- return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
+ return vsp1_subdev_enum_frame_size(subdev, state, fse,
HISTO_MIN_SIZE,
HISTO_MIN_SIZE, HISTO_MAX_SIZE,
HISTO_MAX_SIZE);
@@ -239,13 +239,13 @@ static int histo_get_selection(struct v4l2_subdev *subdev,
}
static int histo_set_crop(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
struct v4l2_mbus_framefmt *format;
/* The crop rectangle must be inside the input frame. */
- format = v4l2_subdev_state_get_format(sd_state, HISTO_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, HISTO_PAD_SINK);
sel->r.left = clamp_t(unsigned int, sel->r.left, 0, format->width - 1);
sel->r.top = clamp_t(unsigned int, sel->r.top, 0, format->height - 1);
sel->r.width = clamp_t(unsigned int, sel->r.width, HISTO_MIN_SIZE,
@@ -254,14 +254,14 @@ static int histo_set_crop(struct v4l2_subdev *subdev,
format->height - sel->r.top);
/* Set the crop rectangle and reset the compose rectangle. */
- *v4l2_subdev_state_get_crop(sd_state, sel->pad) = sel->r;
- *v4l2_subdev_state_get_compose(sd_state, sel->pad) = sel->r;
+ *v4l2_subdev_state_get_crop(state, sel->pad) = sel->r;
+ *v4l2_subdev_state_get_compose(state, sel->pad) = sel->r;
return 0;
}
static int histo_set_compose(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_selection *sel)
{
struct v4l2_rect *compose;
@@ -276,7 +276,7 @@ static int histo_set_compose(struct v4l2_subdev *subdev,
sel->r.left = 0;
sel->r.top = 0;
- crop = v4l2_subdev_state_get_crop(sd_state, sel->pad);
+ crop = v4l2_subdev_state_get_crop(state, sel->pad);
/*
* Clamp the width and height to acceptable values first and then
@@ -301,7 +301,7 @@ static int histo_set_compose(struct v4l2_subdev *subdev,
ratio = 1 << (crop->height * 2 / sel->r.height / 3);
sel->r.height = crop->height / ratio;
- compose = v4l2_subdev_state_get_compose(sd_state, sel->pad);
+ compose = v4l2_subdev_state_get_compose(state, sel->pad);
*compose = sel->r;
return 0;
@@ -323,7 +323,7 @@ static int histo_set_selection(struct v4l2_subdev *subdev,
}
static int histo_set_format(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_format *fmt)
{
struct vsp1_histogram *histo = subdev_to_histo(subdev);
@@ -338,7 +338,7 @@ static int histo_set_format(struct v4l2_subdev *subdev,
return 0;
}
- return vsp1_subdev_set_pad_format(subdev, sd_state, fmt,
+ return vsp1_subdev_set_pad_format(subdev, state, fmt,
histo->formats, histo->num_formats,
HISTO_MIN_SIZE, HISTO_MIN_SIZE,
HISTO_MAX_SIZE, HISTO_MAX_SIZE);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
index 069d3de1797f..0aedaff58019 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
@@ -34,7 +34,7 @@ static inline void vsp1_hsit_write(struct vsp1_hsit *hsit,
*/
static int hsit_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
struct vsp1_hsit *hsit = to_hsit(subdev);
@@ -52,10 +52,10 @@ static int hsit_enum_mbus_code(struct v4l2_subdev *subdev,
}
static int hsit_enum_frame_size(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_frame_size_enum *fse)
{
- return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
+ return vsp1_subdev_enum_frame_size(subdev, state, fse,
HSIT_MIN_SIZE,
HSIT_MIN_SIZE, HSIT_MAX_SIZE,
HSIT_MAX_SIZE);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lif.c b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
index f1fefa2817d4..271ee8fd2b91 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lif.c
@@ -40,28 +40,28 @@ static const unsigned int lif_codes[] = {
};
static int lif_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
- return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, lif_codes,
+ return vsp1_subdev_enum_mbus_code(subdev, 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_state *state,
struct v4l2_subdev_frame_size_enum *fse)
{
- return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
+ return vsp1_subdev_enum_frame_size(subdev, 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_state *state,
struct v4l2_subdev_format *fmt)
{
- return vsp1_subdev_set_pad_format(subdev, sd_state, fmt, lif_codes,
+ return vsp1_subdev_set_pad_format(subdev, state, fmt, lif_codes,
ARRAY_SIZE(lif_codes),
LIF_MIN_SIZE, LIF_MIN_SIZE,
LIF_MAX_SIZE, LIF_MAX_SIZE);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_lut.c b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
index 1063076ad33f..ebe4cb03a7a8 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_lut.c
@@ -99,28 +99,28 @@ static const unsigned int lut_codes[] = {
};
static int lut_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
- return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, lut_codes,
+ return vsp1_subdev_enum_mbus_code(subdev, 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_state *state,
struct v4l2_subdev_frame_size_enum *fse)
{
- return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
+ return vsp1_subdev_enum_frame_size(subdev, 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_state *state,
struct v4l2_subdev_format *fmt)
{
- return vsp1_subdev_set_pad_format(subdev, sd_state, fmt, lut_codes,
+ return vsp1_subdev_set_pad_format(subdev, state, fmt, lut_codes,
ARRAY_SIZE(lut_codes),
LUT_MIN_SIZE, LUT_MIN_SIZE,
LUT_MAX_SIZE, LUT_MAX_SIZE);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index e4edce3bf584..1745e7d714f9 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -21,7 +21,7 @@
*/
static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
static const unsigned int codes[] = {
@@ -39,12 +39,12 @@ static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev,
}
static int vsp1_rwpf_enum_frame_size(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *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,
+ return vsp1_subdev_enum_frame_size(subdev, state, fse,
RWPF_MIN_WIDTH,
RWPF_MIN_HEIGHT, rwpf->max_width,
rwpf->max_height);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
index a5c839381b84..342ca8a28125 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -106,7 +106,7 @@ static const struct v4l2_ctrl_config sru_intensity_control = {
*/
static int sru_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
static const unsigned int codes[] = {
@@ -114,7 +114,7 @@ static int sru_enum_mbus_code(struct v4l2_subdev *subdev,
MEDIA_BUS_FMT_AYUV8_1X32,
};
- return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, codes,
+ return vsp1_subdev_enum_mbus_code(subdev, state, code, codes,
ARRAY_SIZE(codes));
}
@@ -151,7 +151,7 @@ static int sru_enum_frame_size(struct v4l2_subdev *subdev,
}
static void sru_try_format(struct vsp1_sru *sru,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
unsigned int pad, struct v4l2_mbus_framefmt *fmt)
{
struct v4l2_mbus_framefmt *format;
@@ -171,7 +171,7 @@ static void sru_try_format(struct vsp1_sru *sru,
case SRU_PAD_SOURCE:
/* The SRU can't perform format conversion. */
- format = v4l2_subdev_state_get_format(sd_state, SRU_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, SRU_PAD_SINK);
fmt->code = format->code;
/*
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index fd28504d3538..e36720c41143 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -111,7 +111,7 @@ static unsigned int uds_compute_ratio(unsigned int input, unsigned int output)
*/
static int uds_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
static const unsigned int codes[] = {
@@ -119,7 +119,7 @@ static int uds_enum_mbus_code(struct v4l2_subdev *subdev,
MEDIA_BUS_FMT_AYUV8_1X32,
};
- return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, codes,
+ return vsp1_subdev_enum_mbus_code(subdev, state, code, codes,
ARRAY_SIZE(codes));
}
@@ -150,7 +150,7 @@ static int uds_enum_frame_size(struct v4l2_subdev *subdev,
}
static void uds_try_format(struct vsp1_uds *uds,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
unsigned int pad, struct v4l2_mbus_framefmt *fmt)
{
struct v4l2_mbus_framefmt *format;
@@ -170,7 +170,7 @@ static void uds_try_format(struct vsp1_uds *uds,
case UDS_PAD_SOURCE:
/* The UDS scales but can't perform format conversion. */
- format = v4l2_subdev_state_get_format(sd_state, UDS_PAD_SINK);
+ format = v4l2_subdev_state_get_format(state, UDS_PAD_SINK);
fmt->code = format->code;
uds_output_limits(format->width, &minimum, &maximum);
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uif.c b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
index 911543ee702b..9739fefa8260 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uif.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uif.c
@@ -54,28 +54,28 @@ static const unsigned int uif_codes[] = {
};
static int uif_enum_mbus_code(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
- return vsp1_subdev_enum_mbus_code(subdev, sd_state, code, uif_codes,
+ return vsp1_subdev_enum_mbus_code(subdev, 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_state *state,
struct v4l2_subdev_frame_size_enum *fse)
{
- return vsp1_subdev_enum_frame_size(subdev, sd_state, fse,
+ return vsp1_subdev_enum_frame_size(subdev, 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_state *state,
struct v4l2_subdev_format *fmt)
{
- return vsp1_subdev_set_pad_format(subdev, sd_state, fmt, uif_codes,
+ return vsp1_subdev_set_pad_format(subdev, state, fmt, uif_codes,
ARRAY_SIZE(uif_codes),
UIF_MIN_SIZE, UIF_MIN_SIZE,
UIF_MAX_SIZE, UIF_MAX_SIZE);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state
2024-06-19 0:17 [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Laurent Pinchart
` (18 preceding siblings ...)
2024-06-19 0:17 ` [PATCH v2 19/19] media: renesas: vsp1: Rename all v4l2_subdev_state variables to 'state' Laurent Pinchart
@ 2025-06-26 8:30 ` Tomi Valkeinen
2025-06-26 9:07 ` Laurent Pinchart
19 siblings, 1 reply; 25+ messages in thread
From: Tomi Valkeinen @ 2025-06-26 8:30 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: linux-renesas-soc, Sakari Ailus, Jacopo Mondi, Kieran Bingham
Hi Laurent,
On 19/06/2024 03:17, Laurent Pinchart wrote:
> Hello,
>
> This patch series is the second version of the vsp1 driver conversion to
> using the subdev active state. This version suffers from the same main
> lockdep issue as v1 (see below), but I decided to still repost it as the
> first 17 patches can go in already while we figure out how to handle the
> lockdep issue.
>
> The driver is fairly complex and creates many subdevs, exposing them to
> userspace through subdev nodes but also controlling them from within the
> kernel when acting as a backend for the R-Car DU display driver. It is
> thus a good testing ground to validate the subdev active state API.
>
> The first 17 patches are miscellaneous refactoring and cleanups to
> prepare for the actual conversion. In the middle of that is patch 11/19,
> which adds a function to dump a pipeline to the kernel log, which came
> very handy for debugging.
>
> Patch 18/19 is the actual conversion to the active state API. While I
> tried to keep it as small as possible by handling all the refactoring in
> separate patches, it's still the largest one in the series, mostly due
> to the fact that the drivers creates many subdevs. If that's any
> consolation, the diffstat is nice (net removal of 261 lines). Patch
> 19/19 is then an additional cleanup on top.
>
> The good news is that both the VSP test suite ([1]) and the display test
> suite ([2]) pass. The bad news is that lockdep complains quite heavily:
>
> [ 276.810170] ============================================
> [ 276.815489] WARNING: possible recursive locking detected
> [ 276.820813] 6.10.0-rc3-00175-g639ee34a20f1 #803 Not tainted
> [ 276.826401] --------------------------------------------
> [ 276.831724] yavta/1481 is trying to acquire lock:
> [ 276.836442] ffff00000ff26868 (vsp1_entity:531:sd->active_state->lock){+.+.}-{3:3}, at: v4l2_subdev_link_validate+0x140/0x308
> [ 276.847736]
> [ 276.847736] but task is already holding lock:
> [ 276.853581] ffff00000ff27c68 (vsp1_entity:531:sd->active_state->lock){+.+.}-{3:3}, at: v4l2_subdev_link_validate+0x118/0x308
> [ 276.864856]
> [ 276.864856] other info that might help us debug this:
> [ 276.871396] Possible unsafe locking scenario:
> [ 276.871396]
> [ 276.877326] CPU0
> [ 276.879779] ----
> [ 276.882232] lock(vsp1_entity:531:sd->active_state->lock);
> [ 276.887827] lock(vsp1_entity:531:sd->active_state->lock);
> [ 276.893422]
> [ 276.893422] *** DEADLOCK ***
> [ 276.893422]
> [ 276.899349] May be due to missing lock nesting notation
> [ 276.899349]
> [ 276.906145] 3 locks held by yavta/1481:
> [ 276.909996] #0: ffff00000fcaa7a0 (&video->lock){+.+.}-{3:3}, at: __video_do_ioctl+0x19c/0x5b0
> [ 276.918666] #1: ffff00000fc9f418 (&mdev->graph_mutex){+.+.}-{3:3}, at: vsp1_video_streamon+0xec/0x4e8
> [ 276.928031] #2: ffff00000ff27c68 (vsp1_entity:531:sd->active_state->lock){+.+.}-{3:3}, at: v4l2_subdev_link_validate+0x118/0x308
> [ 276.939744]
> [ 276.939744] stack backtrace:
> [ 276.944114] CPU: 1 PID: 1481 Comm: yavta Not tainted 6.10.0-rc3-00175-g639ee34a20f1 #803
> [ 276.952225] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
> [ 276.960157] Call trace:
> [ 276.962616] dump_backtrace+0xa0/0x128
> [ 276.966391] show_stack+0x20/0x38
> [ 276.969723] dump_stack_lvl+0x8c/0xd0
> [ 276.973412] dump_stack+0x1c/0x28
> [ 276.976747] print_deadlock_bug+0x29c/0x3b0
> [ 276.980950] __lock_acquire+0x165c/0x1b80
> [ 276.984978] lock_acquire.part.0+0x168/0x310
> [ 276.989265] lock_acquire+0x70/0x90
> [ 276.992772] __mutex_lock+0x10c/0x4b8
> [ 276.996458] mutex_lock_nested+0x2c/0x40
> [ 277.000396] v4l2_subdev_link_validate+0x140/0x308
> [ 277.005213] __media_pipeline_start+0x880/0xc30
> [ 277.009763] __video_device_pipeline_start+0x48/0x68
> [ 277.014750] vsp1_video_streamon+0x148/0x4e8
> [ 277.019044] v4l_streamon+0x50/0x70
> [ 277.022553] __video_do_ioctl+0x4cc/0x5b0
> [ 277.026583] video_usercopy+0x3c4/0xb90
> [ 277.030438] video_ioctl2+0x20/0x48
> [ 277.033942] v4l2_ioctl+0xa4/0xc8
> [ 277.037278] __arm64_sys_ioctl+0xec/0x118
> [ 277.041310] invoke_syscall+0x68/0x198
> [ 277.045084] el0_svc_common.constprop.0+0x80/0x150
> [ 277.049900] do_el0_svc+0x38/0x50
> [ 277.053237] el0_svc+0x4c/0xc0
> [ 277.056309] el0t_64_sync_handler+0x120/0x130
> [ 277.060685] el0t_64_sync+0x190/0x198
>
> There is no actual deadlock situation caused by the tests, but a lockdep
> class deadlock detection.
>
> Subdev initialization is handled in one helper function for all subdevs
> created by the driver. This causes all locks for the active state being
> created with the same lock class. As a result, when validating links and
> trying to lock both the sink and source states, lockdep complains of
> recursive locking, even if the two locks are different mutex instances.
Sounds familiar =).
> Working around the issue is likely possible. The
> v4l2_subdev_init_finalize() function is actually a macro calling
> __v4l2_subdev_init_finalize() with an explicit lock class. The VSP1
> driver could do the same and push the lock class one layer up, to the
> callers of vsp1_entity_init(). This would solve part of the problem
> only, as the driver creates multiple subdevs of the same type, so
> dynamic allocation of the lock class may be required. That would however
> be a bad solution, or rather not a solution to the actual problem. There
> is a reason why lockdep groups locks in classes, beside just saving
> memory. When locks belonging to different instances of the same object
> type are taken recursively, it is often the sign of a bad design.
>
> Even if we worked around this problem, lockdep would later complain
> about AB-BA locking at link validation time. The VSP1 entities can be
> assembled in a pipeline in any order, so even if the link validation
> helpers are careful to always lock in the sink-source order, the sink
> and source could get swapped.
>
> I believe this calls for a rework of locking for subdev states. The
> option I'm envisioning is to lock all subdevs in a pipeline when
> starting the pipeline, with a new media_pipeline_lock() call just before
> media_pipeline_start(). The link validation helpers would then be called
> with all locks taken, and so would the .s_stream() subdev operations.
> This would simplify locking in drivers as a result, which I think is a
> very desirable side effect. Then, after starting the pipeline, the
> top-level driver would call media_pipeline_unlock(). Similar locking
> would be performed at pipeline stop time, to ensure that the .s_stream()
> operations would also be called with the locks held. Obviously, just
> moving locking around won't fix the lockdep issues, and the second part
> of the fix would be to use ww-mutexes instead of regular mutexes. The
> result would be similar to the implementation of the KMS atomic API,
> giving me some confidence that it goes in the right direction. An
> additional difficulty, however, is that we need to lock both the subdev
> active state and the control handler with the same lock.
This has been the long term plan, but perhaps it's not so far off in the
future, then...
But should the control state be merged to the active state first? I fear
that's a large amount of work in itself, though, but managing the active
state and control state locking separately with the scheme you outline
sounds complex.
Or can we do a new requirement for active-state-enabled subdevs: the
control handler _must_ use the same lock as the active state (which, I
think, is often done already). Would that help?
Or as a temporary solution... If there's a central place in the vsp1
infrastructure, can we have a single (shared) state lock for all the
vsp1 subdevs? That would mean taking the same lock multiple times, but
somehow I recall that a similar thing was already done in some of the
drivers.
So I agree with the goal. But it would be nice to not block this series
until all the work is done. Or can we annotate the locking for this
particular case so that lockdep doesn't care about vsp1 locks?
Tomi
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state
2025-06-26 8:30 ` [PATCH v2 00/19] media: renesas: vsp1: Conversion to subdev active state Tomi Valkeinen
@ 2025-06-26 9:07 ` Laurent Pinchart
0 siblings, 0 replies; 25+ messages in thread
From: Laurent Pinchart @ 2025-06-26 9:07 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-media, linux-renesas-soc, Sakari Ailus, Jacopo Mondi,
Kieran Bingham
Hi Tomi,
On Thu, Jun 26, 2025 at 11:30:44AM +0300, Tomi Valkeinen wrote:
> On 19/06/2024 03:17, Laurent Pinchart wrote:
> > Hello,
> >
> > This patch series is the second version of the vsp1 driver conversion to
> > using the subdev active state. This version suffers from the same main
> > lockdep issue as v1 (see below), but I decided to still repost it as the
> > first 17 patches can go in already while we figure out how to handle the
> > lockdep issue.
> >
> > The driver is fairly complex and creates many subdevs, exposing them to
> > userspace through subdev nodes but also controlling them from within the
> > kernel when acting as a backend for the R-Car DU display driver. It is
> > thus a good testing ground to validate the subdev active state API.
> >
> > The first 17 patches are miscellaneous refactoring and cleanups to
> > prepare for the actual conversion. In the middle of that is patch 11/19,
> > which adds a function to dump a pipeline to the kernel log, which came
> > very handy for debugging.
> >
> > Patch 18/19 is the actual conversion to the active state API. While I
> > tried to keep it as small as possible by handling all the refactoring in
> > separate patches, it's still the largest one in the series, mostly due
> > to the fact that the drivers creates many subdevs. If that's any
> > consolation, the diffstat is nice (net removal of 261 lines). Patch
> > 19/19 is then an additional cleanup on top.
> >
> > The good news is that both the VSP test suite ([1]) and the display test
> > suite ([2]) pass. The bad news is that lockdep complains quite heavily:
> >
> > [ 276.810170] ============================================
> > [ 276.815489] WARNING: possible recursive locking detected
> > [ 276.820813] 6.10.0-rc3-00175-g639ee34a20f1 #803 Not tainted
> > [ 276.826401] --------------------------------------------
> > [ 276.831724] yavta/1481 is trying to acquire lock:
> > [ 276.836442] ffff00000ff26868 (vsp1_entity:531:sd->active_state->lock){+.+.}-{3:3}, at: v4l2_subdev_link_validate+0x140/0x308
> > [ 276.847736]
> > [ 276.847736] but task is already holding lock:
> > [ 276.853581] ffff00000ff27c68 (vsp1_entity:531:sd->active_state->lock){+.+.}-{3:3}, at: v4l2_subdev_link_validate+0x118/0x308
> > [ 276.864856]
> > [ 276.864856] other info that might help us debug this:
> > [ 276.871396] Possible unsafe locking scenario:
> > [ 276.871396]
> > [ 276.877326] CPU0
> > [ 276.879779] ----
> > [ 276.882232] lock(vsp1_entity:531:sd->active_state->lock);
> > [ 276.887827] lock(vsp1_entity:531:sd->active_state->lock);
> > [ 276.893422]
> > [ 276.893422] *** DEADLOCK ***
> > [ 276.893422]
> > [ 276.899349] May be due to missing lock nesting notation
> > [ 276.899349]
> > [ 276.906145] 3 locks held by yavta/1481:
> > [ 276.909996] #0: ffff00000fcaa7a0 (&video->lock){+.+.}-{3:3}, at: __video_do_ioctl+0x19c/0x5b0
> > [ 276.918666] #1: ffff00000fc9f418 (&mdev->graph_mutex){+.+.}-{3:3}, at: vsp1_video_streamon+0xec/0x4e8
> > [ 276.928031] #2: ffff00000ff27c68 (vsp1_entity:531:sd->active_state->lock){+.+.}-{3:3}, at: v4l2_subdev_link_validate+0x118/0x308
> > [ 276.939744]
> > [ 276.939744] stack backtrace:
> > [ 276.944114] CPU: 1 PID: 1481 Comm: yavta Not tainted 6.10.0-rc3-00175-g639ee34a20f1 #803
> > [ 276.952225] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
> > [ 276.960157] Call trace:
> > [ 276.962616] dump_backtrace+0xa0/0x128
> > [ 276.966391] show_stack+0x20/0x38
> > [ 276.969723] dump_stack_lvl+0x8c/0xd0
> > [ 276.973412] dump_stack+0x1c/0x28
> > [ 276.976747] print_deadlock_bug+0x29c/0x3b0
> > [ 276.980950] __lock_acquire+0x165c/0x1b80
> > [ 276.984978] lock_acquire.part.0+0x168/0x310
> > [ 276.989265] lock_acquire+0x70/0x90
> > [ 276.992772] __mutex_lock+0x10c/0x4b8
> > [ 276.996458] mutex_lock_nested+0x2c/0x40
> > [ 277.000396] v4l2_subdev_link_validate+0x140/0x308
> > [ 277.005213] __media_pipeline_start+0x880/0xc30
> > [ 277.009763] __video_device_pipeline_start+0x48/0x68
> > [ 277.014750] vsp1_video_streamon+0x148/0x4e8
> > [ 277.019044] v4l_streamon+0x50/0x70
> > [ 277.022553] __video_do_ioctl+0x4cc/0x5b0
> > [ 277.026583] video_usercopy+0x3c4/0xb90
> > [ 277.030438] video_ioctl2+0x20/0x48
> > [ 277.033942] v4l2_ioctl+0xa4/0xc8
> > [ 277.037278] __arm64_sys_ioctl+0xec/0x118
> > [ 277.041310] invoke_syscall+0x68/0x198
> > [ 277.045084] el0_svc_common.constprop.0+0x80/0x150
> > [ 277.049900] do_el0_svc+0x38/0x50
> > [ 277.053237] el0_svc+0x4c/0xc0
> > [ 277.056309] el0t_64_sync_handler+0x120/0x130
> > [ 277.060685] el0t_64_sync+0x190/0x198
> >
> > There is no actual deadlock situation caused by the tests, but a lockdep
> > class deadlock detection.
> >
> > Subdev initialization is handled in one helper function for all subdevs
> > created by the driver. This causes all locks for the active state being
> > created with the same lock class. As a result, when validating links and
> > trying to lock both the sink and source states, lockdep complains of
> > recursive locking, even if the two locks are different mutex instances.
>
> Sounds familiar =).
>
> > Working around the issue is likely possible. The
> > v4l2_subdev_init_finalize() function is actually a macro calling
> > __v4l2_subdev_init_finalize() with an explicit lock class. The VSP1
> > driver could do the same and push the lock class one layer up, to the
> > callers of vsp1_entity_init(). This would solve part of the problem
> > only, as the driver creates multiple subdevs of the same type, so
> > dynamic allocation of the lock class may be required. That would however
> > be a bad solution, or rather not a solution to the actual problem. There
> > is a reason why lockdep groups locks in classes, beside just saving
> > memory. When locks belonging to different instances of the same object
> > type are taken recursively, it is often the sign of a bad design.
> >
> > Even if we worked around this problem, lockdep would later complain
> > about AB-BA locking at link validation time. The VSP1 entities can be
> > assembled in a pipeline in any order, so even if the link validation
> > helpers are careful to always lock in the sink-source order, the sink
> > and source could get swapped.
> >
> > I believe this calls for a rework of locking for subdev states. The
> > option I'm envisioning is to lock all subdevs in a pipeline when
> > starting the pipeline, with a new media_pipeline_lock() call just before
> > media_pipeline_start(). The link validation helpers would then be called
> > with all locks taken, and so would the .s_stream() subdev operations.
> > This would simplify locking in drivers as a result, which I think is a
> > very desirable side effect. Then, after starting the pipeline, the
> > top-level driver would call media_pipeline_unlock(). Similar locking
> > would be performed at pipeline stop time, to ensure that the .s_stream()
> > operations would also be called with the locks held. Obviously, just
> > moving locking around won't fix the lockdep issues, and the second part
> > of the fix would be to use ww-mutexes instead of regular mutexes. The
> > result would be similar to the implementation of the KMS atomic API,
> > giving me some confidence that it goes in the right direction. An
> > additional difficulty, however, is that we need to lock both the subdev
> > active state and the control handler with the same lock.
>
> This has been the long term plan, but perhaps it's not so far off in the
> future, then...
Sounds like I need to give it a try then.
> But should the control state be merged to the active state first? I fear
> that's a large amount of work in itself, though, but managing the active
> state and control state locking separately with the scheme you outline
> sounds complex.
>
> Or can we do a new requirement for active-state-enabled subdevs: the
> control handler _must_ use the same lock as the active state (which, I
> think, is often done already). Would that help?
I think that would help, and I also think that drivers are largely doing
that already. I don't think we document it though.
> Or as a temporary solution... If there's a central place in the vsp1
> infrastructure, can we have a single (shared) state lock for all the
> vsp1 subdevs? That would mean taking the same lock multiple times, but
> somehow I recall that a similar thing was already done in some of the
> drivers.
I'll check that.
> So I agree with the goal. But it would be nice to not block this series
> until all the work is done. Or can we annotate the locking for this
> particular case so that lockdep doesn't care about vsp1 locks?
All the preparatory patches (01/19 to 17/19) have been merged, it's only
the last two that are blocked. I'm not sure about lock annotation, I'll
give it a look.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 25+ messages in thread