On 01/03/2023 11:40, Jacopo Mondi wrote:
Hi Tomi
On Tue, Feb 28, 2023 at 07:16:19PM +0200, Tomi Valkeinen wrote:
CAL uses get_frame_desc to get the VC and DT for the incoming CSI-2
stream, but does it in a bit hacky way. Clean this up by implementing
.get_frame_desc to camera-rx, and calling that from cal.c.
No functional change intended.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
drivers/media/platform/ti/cal/cal-camerarx.c | 62 +++++++++++---------
drivers/media/platform/ti/cal/cal.c | 30 ++++------
drivers/media/platform/ti/cal/cal.h | 2 -
3 files changed, 47 insertions(+), 47 deletions(-)
diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
index 3dfcb276d367..95e0ad59a39b 100644
--- a/drivers/media/platform/ti/cal/cal-camerarx.c
+++ b/drivers/media/platform/ti/cal/cal-camerarx.c
@@ -589,33 +589,6 @@ static int cal_camerarx_parse_dt(struct cal_camerarx *phy)
return ret;
}
-int cal_camerarx_get_remote_frame_desc(struct cal_camerarx *phy,
- struct v4l2_mbus_frame_desc *desc)
-{
- struct media_pad *pad;
- int ret;
-
- if (!phy->source)
- return -EPIPE;
-
- pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
- if (!pad)
- return -EPIPE;
-
- ret = v4l2_subdev_call(phy->source, pad, get_frame_desc, pad->index,
- desc);
- if (ret)
- return ret;
-
- if (desc->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
- dev_err(phy->cal->dev,
- "Frame descriptor does not describe CSI-2 link");
- return -EINVAL;
- }
-
- return 0;
-}
-
/* ------------------------------------------------------------------
* V4L2 Subdev Operations
* ------------------------------------------------------------------
@@ -772,6 +745,40 @@ static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
return cal_camerarx_sd_set_fmt(sd, state, &format);
}
+static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+ struct v4l2_mbus_frame_desc *fd)
+{
+ struct cal_camerarx *phy = to_cal_camerarx(sd);
+ struct v4l2_mbus_frame_desc remote_desc;
+ const struct media_pad *remote_pad;
+ int ret;
+
+ remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
+ if (!remote_pad)
+ return -EPIPE;
+
+ ret = v4l2_subdev_call(phy->source, pad, get_frame_desc,
+ remote_pad->index, &remote_desc);
+ if (ret)
+ return ret;
+
+ if (remote_desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
+ dev_err(phy->cal->dev,
+ "Frame descriptor does not describe CSI-2 link");
+ return -EINVAL;
+ }
+
+ if (remote_desc.num_entries > 1)
+ dev_dbg(phy->cal->dev,
+ "Multiple streams not supported in remote frame descriptor, using the first one\n");
Maybe WARN_ONCE, but doesn't really matter as I presume this will go
away in next patch
+
+ fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
Is the bus between CAL and camera-rx a CSI-2 bus ??
No, it's a custom internal bus.
As this mostly serves to transport to CAL the DT and VC, I guess it's
fine
Yes, we use it to transport CSI-2 data, and mainly just the DT and VC.
The SW architecture doesn't really match the HW, but I think it works
quite fine. E.g. there's really just a single output from each of the
PHYs, which go to the CAL block (so CAL would have two inputs), and CAL
contains a processing pipeline, and at the end it has the DMAs and
memory connection.
I don't know if trying to design the SW like that would help any, and
besides, CAL is an old driver so the legacy drag is there. The current
design is an extension to the older designs.
Here, we could of course drop the .get_frame_desc from camera-rx, and
implement (more or less identical) function which the cal.c could call.
In some ways that would be better, but then again, I think the
.get_frame_desc works here quite nicely, as cal.c can ask the frame desc
for a single output pad (i.e. the stream which the context in question
is interested in).
Reviewed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
Thanks!
Tomi
