Re: [PATCH v5 06/10] media: rcar-csi2: Add .get_frame_desc op
From: Tomi Valkeinen
Date: Tue Jun 16 2026 - 09:12:42 EST
Hi,
On 16/06/2026 15:30, Laurent Pinchart wrote:
On Tue, Jun 16, 2026 at 02:30:05PM +0300, Tomi Valkeinen wrote:
On 18/03/2026 23:16, Laurent Pinchart wrote:
On Wed, Mar 11, 2026 at 03:53:19PM +0200, Tomi Valkeinen wrote:
Add v4l2_subdev_pad_ops.get_frame_desc() implementation.
We also implement a fallback for the case where the upstream subdevice
does not implement .get_frame_desc. It assumes a single stream with VC =
0 and DT based on the configured stream mbus format.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
---
drivers/media/platform/renesas/rcar-csi2.c | 70 ++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
index ad62c95c8f9a..b8baf7c65e90 100644
--- a/drivers/media/platform/renesas/rcar-csi2.c
+++ b/drivers/media/platform/renesas/rcar-csi2.c
@@ -1935,12 +1935,82 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
return 0;
}
+static int rcsi2_get_frame_desc_fallback(struct v4l2_subdev *sd,
+ unsigned int pad,
+ struct v4l2_mbus_frame_desc *fd)
+{
+ struct v4l2_subdev_route *route;
+ const struct rcar_csi2_format *format;
+ struct v4l2_subdev_state *state;
+ struct v4l2_mbus_framefmt *fmt;
+ int ret = 0;
+
+ state = v4l2_subdev_lock_and_get_active_state(sd);
+
+ if (state->routing.num_routes != 1) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ route = &state->routing.routes[0];
+
+ if (route->source_pad != pad) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
+ route->sink_stream);
+ if (!fmt) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ format = rcsi2_code_to_fmt(fmt->code);
+ if (!format) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ fd->num_entries = 1;
+ fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+ fd->entry[0].stream = route->source_stream;
+ fd->entry[0].pixelcode = fmt->code;
+ fd->entry[0].bus.csi2.vc = 0;
+ fd->entry[0].bus.csi2.dt = format->datatype;
+
+out:
+ v4l2_subdev_unlock_state(state);
+
+ return ret;
+}
+
+static int rcsi2_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+ struct v4l2_mbus_frame_desc *fd)
+{
+ struct rcar_csi2 *priv = sd_to_csi2(sd);
+ int ret;
+
+ if (WARN_ON(!priv->info->use_isp))
+ return -ENOTTY;
Why is that, can't the get frame desc operation be supported on Gen3 ?
It can, and it is, in this patch. The thing here is that
rcsi2_get_frame_desc() is the implementation for
v4l2_subdev_pad_ops.get_frame_desc(). On gen4, csisp calls it, but on
gen3, there's no one to call it as on gen3 the csi2 does the demuxing.
So the above check is just a "yell if our drivers do a totally wrong thing".
A comment would be useful.
Sure, I'll add one here.
+
+ if (WARN_ON(pad != RCAR_CSI2_SOURCE_VC0))
+ return -EINVAL;
+
+ ret = v4l2_subdev_get_frame_desc_passthrough(sd, pad, fd);
+ if (ret == -ENOIOCTLCMD)
+ ret = rcsi2_get_frame_desc_fallback(sd, pad, fd);
A dev_warn_once() would be good here, to get people to fix the source
device driver.
Perhaps at some point, but do we want to add it already?
Hmm, actually, this will go away with Sakari's series that adds
framework level fallback handling, so I think warning now about a thing
that is no longer a thing in the future serves no purpose.
Or you could consider that adding a dev_warn_once() isn't an issue as
the code will be replaced soon :-) Up to you.
I meant that if we add dev_warn here, it'll start printing a warning "to get people to fix the source driver", but with Sakari's series that "fix" is not needed at all, so the people would end up doing unnecessary work.
Tomi