* [PATCH v2 0/4] media: ti: cal: Streams support
@ 2023-02-28 17:16 Tomi Valkeinen
2023-02-28 17:16 ` [PATCH v2 1/4] media: ti: cal: Clean up mbus formats uses Tomi Valkeinen
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Tomi Valkeinen @ 2023-02-28 17:16 UTC (permalink / raw)
To: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus
Cc: Kieran Bingham, Jai Luthra, Vaishnav Achath, Tomi Valkeinen
Hi,
This series is based on Laurent's "media: Zero-initialize structures
passed to subdev pad ops" series and my two patches adding
V4L2_SUBDEV_ROUTING_* flags.
Changes to v1:
- Drop the 2X8 formats and some other unsupported formats, and switch to
1X16 formats where appropriate.
- New commit "Implement get_frame_desc for camera-rx" which adds a
proper .get_frame_desc op for camerarx. This allows a bit simpler
code in the patch that adds the multiplexed streams.
- Use V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING flag
- Simplified the code in places, where we, e.g., know the pad is the
single camera-rx sink pad, so instead of using a pad variable passed
around we just use the CAL_CAMERARX_PAD_SINK.
Range-diff to v1 below.
Tomi
Tomi Valkeinen (4):
media: ti: cal: Clean up mbus formats uses
media: ti: cal: Use subdev state
media: ti: cal: Implement get_frame_desc for camera-rx
media: ti: cal: Add multiplexed streams support
drivers/media/platform/ti/cal/cal-camerarx.c | 405 ++++++++++++-------
drivers/media/platform/ti/cal/cal-video.c | 142 +++++--
drivers/media/platform/ti/cal/cal.c | 107 +++--
drivers/media/platform/ti/cal/cal.h | 13 +-
4 files changed, 409 insertions(+), 258 deletions(-)
Range-diff against v1:
1: 2a757d193135 < -: ------------ media: ti: cal: Add support for 1X16 mbus formats
-: ------------ > 1: c4f423c12a1a media: ti: cal: Clean up mbus formats uses
2: 93d34b7c33c0 ! 2: ca88904ee5d5 media: ti: cal: Use subdev state
@@ drivers/media/platform/ti/cal/cal-camerarx.c: void cal_camerarx_destroy(struct c
}
## drivers/media/platform/ti/cal/cal-video.c ##
-@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_video_check_format(struct cal_ctx *ctx)
+@@ drivers/media/platform/ti/cal/cal-video.c: static void cal_release_buffers(struct cal_ctx *ctx,
+ static int cal_video_check_format(struct cal_ctx *ctx)
{
- const struct cal_format_info *rx_fmtinfo;
const struct v4l2_mbus_framefmt *format;
+ struct v4l2_subdev_state *state;
struct media_pad *remote_pad;
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_video_check_format(str
+ goto out;
+ }
- rx_fmtinfo = cal_format_by_code(format->code);
-- if (!rx_fmtinfo)
-- return -EINVAL;
-+ if (!rx_fmtinfo) {
-+ ret = -EINVAL;
-+ goto out;
-+ }
-
- if (ctx->fmtinfo->fourcc != rx_fmtinfo->fourcc ||
+ if (ctx->fmtinfo->code != format->code ||
ctx->v_fmt.fmt.pix.height != format->height ||
ctx->v_fmt.fmt.pix.width != format->width ||
- ctx->v_fmt.fmt.pix.field != format->field)
-: ------------ > 3: 15a8758f536a media: ti: cal: Implement get_frame_desc for camera-rx
3: bbd8fcfa857f ! 4: 1b698d8bb0ee media: ti: cal: add multiplexed streams support
@@ Metadata
Author: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
## Commit message ##
- media: ti: cal: add multiplexed streams support
+ media: ti: cal: Add multiplexed streams support
Add routing and stream_config support to CAL driver.
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static s64 cal_camerarx_get_ext_li
u32 bpp;
s64 freq;
-- state = v4l2_subdev_get_locked_active_state(&phy->subdev);
+ /*
+ * v4l2_get_link_freq() uses V4L2_CID_LINK_FREQ first, and falls back
+ * to V4L2_CID_PIXEL_RATE if V4L2_CID_LINK_FREQ is not available.
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static s64 cal_camerarx_get_ext_li
+ * causes v4l2_get_link_freq() to return an error if it falls back to
+ * V4L2_CID_PIXEL_RATE.
+ */
++
+ state = v4l2_subdev_get_locked_active_state(&phy->subdev);
- fmt = v4l2_subdev_get_pad_format(&phy->subdev, state, CAL_CAMERARX_PAD_SINK);
-+ state = v4l2_subdev_get_locked_active_state(&phy->subdev);
-
-- fmtinfo = cal_format_by_code(fmt->code);
-- if (!fmtinfo)
-+ if (state->routing.num_routes == 0)
- return -EINVAL;
-
-- bpp = fmtinfo->bpp;
+ if (state->routing.num_routes > 1) {
+ bpp = 0;
+ } else {
-+ const struct cal_format_info *fmtinfo;
+ struct v4l2_subdev_route *route = &state->routing.routes[0];
++ const struct cal_format_info *fmtinfo;
+ struct v4l2_mbus_framefmt *fmt;
-+
+
+- fmtinfo = cal_format_by_code(fmt->code);
+- if (!fmtinfo)
+- return -EINVAL;
+ fmt = v4l2_subdev_state_get_stream_format(
+ state, route->sink_pad, route->sink_stream);
-+
+
+- bpp = fmtinfo->bpp;
+ fmtinfo = cal_format_by_code(fmt->code);
+ if (!fmtinfo)
+ return -EINVAL;
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static void cal_camerarx_ppi_disab
}
-static int cal_camerarx_start(struct cal_camerarx *phy)
-+static int cal_camerarx_start(struct cal_camerarx *phy, u32 pad, u32 stream)
++static int cal_camerarx_start(struct cal_camerarx *phy, u32 sink_stream)
{
+ struct media_pad *remote_pad;
s64 link_freq;
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static void cal_camerarx_ppi_disab
u32 val;
int ret;
-+ remote_pad = media_pad_remote_pad_first(&phy->pads[pad]);
++ remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
+
- if (phy->enable_count > 0) {
- phy->enable_count++;
++ /*
++ * We need to enable the PHY hardware when enabling the first stream,
++ * but for the following streams we just propagate the enable_streams
++ * to the source.
++ */
+
+ if (phy->enable_count > 0) {
+ ret = v4l2_subdev_enable_streams(phy->source, remote_pad->index,
-+ BIT(stream));
++ BIT(sink_stream));
+ if (ret) {
-+ phy->enable_count--;
-+
+ phy_err(phy, "enable streams failed in source: %d\n", ret);
+ return ret;
+ }
++
+ phy->enable_count++;
+
return 0;
}
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static int cal_camerarx_start(stru
* CSI-2 PHY reset to complete.
*/
- ret = v4l2_subdev_call(phy->source, video, s_stream, 1);
-+
+ ret = v4l2_subdev_enable_streams(phy->source, remote_pad->index,
-+ BIT(stream));
++ BIT(sink_stream));
if (ret) {
v4l2_subdev_call(phy->source, core, s_power, 0);
cal_camerarx_disable_irqs(phy);
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static int cal_camerarx_start(stru
}
-static void cal_camerarx_stop(struct cal_camerarx *phy)
-+static void cal_camerarx_stop(struct cal_camerarx *phy, u32 pad, u32 stream)
++static void cal_camerarx_stop(struct cal_camerarx *phy, u32 sink_stream)
{
+ struct media_pad *remote_pad;
int ret;
- if (--phy->enable_count > 0)
-+ remote_pad = media_pad_remote_pad_first(&phy->pads[pad]);
++ remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
+
+ if (--phy->enable_count > 0) {
+ ret = v4l2_subdev_disable_streams(phy->source,
+ remote_pad->index,
-+ BIT(stream));
++ BIT(sink_stream));
+ if (ret)
+ phy_err(phy, "stream off failed in subdev\n");
+
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static void cal_camerarx_stop(stru
- if (v4l2_subdev_call(phy->source, video, s_stream, 0))
+ ret = v4l2_subdev_disable_streams(phy->source, remote_pad->index,
-+ BIT(stream));
++ BIT(sink_stream));
+ if (ret)
phy_err(phy, "stream off failed in subdev\n");
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static inline struct cal_camerarx
struct cal_camerarx *phy = to_cal_camerarx(sd);
- struct v4l2_subdev_state *state;
- int ret = 0;
-+ u32 other_pad, other_stream;
++ u32 sink_stream;
+ int ret;
- state = v4l2_subdev_lock_and_get_active_state(sd);
-+ if (WARN_ON(streams_mask != 1))
-+ return -EINVAL;
++ ret = v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0,
++ NULL, &sink_stream);
++ if (ret)
++ return ret;
- if (enable)
- ret = cal_camerarx_start(phy);
- else
- cal_camerarx_stop(phy);
-+ ret = v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0,
-+ &other_pad, &other_stream);
-+ if (ret)
-+ return ret;
-
-- v4l2_subdev_unlock_state(state);
-+ return cal_camerarx_start(phy, other_pad, other_stream);
++ return cal_camerarx_start(phy, sink_stream);
+}
-- return ret;
+- v4l2_subdev_unlock_state(state);
+static int cal_camerarx_sd_disable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ u32 pad, u64 streams_mask)
+{
+ struct cal_camerarx *phy = to_cal_camerarx(sd);
-+ u32 other_pad, other_stream;
++ u32 sink_stream;
+ int ret;
-+
-+ if (WARN_ON(streams_mask != 1))
-+ return -EINVAL;
-+
+
+- return ret;
+ ret = v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0,
-+ &other_pad, &other_stream);
++ NULL, &sink_stream);
+ if (ret)
+ return ret;
+
-+ cal_camerarx_stop(phy, other_pad, other_stream);
++ cal_camerarx_stop(phy, sink_stream);
+
+ return 0;
}
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static int cal_camerarx_sd_set_fmt
return 0;
}
-+static int _cal_camerarx_sd_set_routing(struct v4l2_subdev *sd,
-+ struct v4l2_subdev_state *state,
-+ struct v4l2_subdev_krouting *routing)
++static int cal_camerarx_set_routing(struct v4l2_subdev *sd,
++ struct v4l2_subdev_state *state,
++ struct v4l2_subdev_krouting *routing)
+{
+ static const struct v4l2_mbus_framefmt format = {
+ .width = 640,
+ .height = 480,
-+ .code = MEDIA_BUS_FMT_UYVY8_2X8,
++ .code = MEDIA_BUS_FMT_UYVY8_1X16,
+ .field = V4L2_FIELD_NONE,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .ycbcr_enc = V4L2_YCBCR_ENC_601,
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static int cal_camerarx_sd_set_fmt
+ };
+ int ret;
+
-+ ret = v4l2_subdev_routing_validate(sd, routing, V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
++ ret = v4l2_subdev_routing_validate(sd, routing,
++ V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 |
++ V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING);
+ if (ret)
+ return ret;
+
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static int cal_camerarx_sd_set_fmt
+ enum v4l2_subdev_format_whence which,
+ struct v4l2_subdev_krouting *routing)
+{
-+ return _cal_camerarx_sd_set_routing(sd, state, routing);
++ return cal_camerarx_set_routing(sd, state, routing);
+}
+
static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static int cal_camerarx_sd_set_fmt
- .format = {
- .width = 640,
- .height = 480,
-- .code = MEDIA_BUS_FMT_UYVY8_2X8,
+- .code = MEDIA_BUS_FMT_UYVY8_1X16,
- .field = V4L2_FIELD_NONE,
- .colorspace = V4L2_COLORSPACE_SRGB,
- .ycbcr_enc = V4L2_YCBCR_ENC_601,
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static int cal_camerarx_sd_set_fmt
- return cal_camerarx_sd_set_fmt(sd, state, &format);
+ /* Initialize routing to single route to the fist source pad */
-+ return _cal_camerarx_sd_set_routing(sd, state, &routing);
++ return cal_camerarx_set_routing(sd, state, &routing);
}
+ static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+@@ drivers/media/platform/ti/cal/cal-camerarx.c: static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+ struct cal_camerarx *phy = to_cal_camerarx(sd);
+ struct v4l2_mbus_frame_desc remote_desc;
+ const struct media_pad *remote_pad;
++ struct v4l2_subdev_state *state;
++ u32 sink_stream;
++ unsigned int i;
+ int ret;
+
++ state = v4l2_subdev_lock_and_get_active_state(sd);
++
++ ret = v4l2_subdev_routing_find_opposite_end(&state->routing,
++ pad, 0,
++ NULL, &sink_stream);
++ if (ret)
++ goto out_unlock;
++
+ remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
+- if (!remote_pad)
+- return -EPIPE;
++ if (!remote_pad) {
++ ret = -EPIPE;
++ goto out_unlock;
++ }
+
+ ret = v4l2_subdev_call(phy->source, pad, get_frame_desc,
+ remote_pad->index, &remote_desc);
+ if (ret)
+- return ret;
++ goto out_unlock;
+
+ 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;
++ ret = -EINVAL;
++ goto out_unlock;
++ }
++
++ for (i = 0; i < remote_desc.num_entries; i++) {
++ if (remote_desc.entry[i].stream == sink_stream)
++ break;
+ }
+
+- if (remote_desc.num_entries > 1)
+- dev_dbg(phy->cal->dev,
+- "Multiple streams not supported in remote frame descriptor, using the first one\n");
++ if (i == remote_desc.num_entries) {
++ ret = -EINVAL;
++ goto out_unlock;
++ }
+
+ fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+ fd->num_entries = 1;
+- fd->entry[0] = remote_desc.entry[0];
++ fd->entry[0] = remote_desc.entry[i];
+
+- return 0;
+-}
++out_unlock:
++ v4l2_subdev_unlock_state(state);
+
-static const struct v4l2_subdev_video_ops cal_camerarx_video_ops = {
- .s_stream = cal_camerarx_sd_s_stream,
-};
--
++ return ret;
++}
+
static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
+ .enable_streams = cal_camerarx_sd_enable_streams,
+ .disable_streams = cal_camerarx_sd_disable_streams,
@@ drivers/media/platform/ti/cal/cal-camerarx.c: static int cal_camerarx_sd_set_fmt
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = cal_camerarx_sd_set_fmt,
+ .set_routing = cal_camerarx_sd_set_routing,
+ .get_frame_desc = cal_camerarx_get_frame_desc,
};
static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
@@ drivers/media/platform/ti/cal/cal-camerarx.c: struct cal_camerarx *cal_camerarx_
snprintf(sd->name, sizeof(sd->name), "CAMERARX%u", instance);
sd->dev = cal->dev;
- phy->pads[CAL_CAMERARX_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
-+
- for (i = CAL_CAMERARX_PAD_FIRST_SOURCE; i < CAL_CAMERARX_NUM_PADS; ++i)
- phy->pads[i].flags = MEDIA_PAD_FL_SOURCE;
- sd->entity.ops = &cal_camerarx_media_ops;
-@@ drivers/media/platform/ti/cal/cal-camerarx.c: void cal_camerarx_destroy(struct cal_camerarx *phy)
- return;
-
- v4l2_device_unregister_subdev(&phy->subdev);
-+
- v4l2_subdev_cleanup(&phy->subdev);
-+
- media_entity_cleanup(&phy->subdev.entity);
- of_node_put(phy->source_ep_node);
- of_node_put(phy->source_node);
## drivers/media/platform/ti/cal/cal-video.c ##
@@ drivers/media/platform/ti/cal/cal-video.c: static int __subdev_get_format(struct cal_ctx *ctx,
- {
- struct v4l2_subdev_format sd_fmt;
+ .pad = 0,
+ };
struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
+ struct v4l2_subdev *sd = ctx->phy->source;
int ret;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- sd_fmt.pad = 0;
-
- ret = v4l2_subdev_call(ctx->phy->source, pad, get_fmt, NULL, &sd_fmt);
+ ret = v4l2_subdev_call_state_active(sd, pad, get_fmt, &sd_fmt);
if (ret)
return ret;
@@ drivers/media/platform/ti/cal/cal-video.c: static int __subdev_set_format(struct cal_ctx *ctx,
- {
- struct v4l2_subdev_format sd_fmt;
+ .pad = 0,
+ };
struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
+ struct v4l2_subdev *sd = ctx->phy->source;
int ret;
- sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- sd_fmt.pad = 0;
*mbus_fmt = *fmt;
- ret = v4l2_subdev_call(ctx->phy->source, pad, set_fmt, NULL, &sd_fmt);
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_legacy_try_fmt_vid_cap
struct cal_ctx *ctx = video_drvdata(file);
+ struct v4l2_subdev *sd = ctx->phy->source;
const struct cal_format_info *fmtinfo;
- struct v4l2_subdev_frame_size_enum fse;
- int found;
+ struct v4l2_subdev_frame_size_enum fse = {
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_legacy_try_fmt_vid_cap(struct file *file, void *priv,
for (fse.index = 0; ; fse.index++) {
int ret;
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_legacy_enum_framesizes
struct cal_ctx *ctx = video_drvdata(file);
+ struct v4l2_subdev *sd = ctx->phy->source;
const struct cal_format_info *fmtinfo;
- struct v4l2_subdev_frame_size_enum fse;
- int ret;
+ struct v4l2_subdev_frame_size_enum fse = {
+ .index = fsize->index,
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_legacy_enum_framesizes(struct file *file, void *fh,
+
fse.code = fmtinfo->code;
- fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- ret = v4l2_subdev_call(ctx->phy->source, pad, enum_frame_size, NULL,
- &fse);
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_legacy_enum_frameinter
fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_video_check_format(struct cal_ctx *ctx)
{
- const struct cal_format_info *rx_fmtinfo;
const struct v4l2_mbus_framefmt *format;
-- struct v4l2_subdev_state *state;
- struct media_pad *remote_pad;
-+ struct v4l2_subdev_state *state;
+ struct v4l2_subdev_state *state;
+- struct media_pad *remote_pad;
++ struct media_pad *phy_source_pad;
int ret = 0;
- remote_pad = media_pad_remote_pad_first(&ctx->pad);
-@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_video_check_format(struct cal_ctx *ctx)
+- remote_pad = media_pad_remote_pad_first(&ctx->pad);
+- if (!remote_pad)
++ phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
++ if (!phy_source_pad)
+ return -ENODEV;
state = v4l2_subdev_lock_and_get_active_state(&ctx->phy->subdev);
- format = v4l2_subdev_get_pad_format(&ctx->phy->subdev, state, remote_pad->index);
-+ format = v4l2_subdev_state_get_stream_format(state, remote_pad->index,
-+ 0);
++ format = v4l2_subdev_state_get_stream_format(state,
++ phy_source_pad->index, 0);
if (!format) {
ret = -EINVAL;
goto out;
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_video_check_format(str
static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
{
struct cal_ctx *ctx = vb2_get_drv_priv(vq);
-+ struct media_pad *remote_pad;
++ struct media_pad *phy_source_pad;
struct cal_buffer *buf;
dma_addr_t addr;
int ret;
-+ remote_pad = media_pad_remote_pad_first(&ctx->pad);
-+ if (!remote_pad) {
++ phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
++ if (!phy_source_pad) {
+ ctx_err(ctx, "Context not connected\n");
+ ret = -ENODEV;
+ goto error_release_buffers;
+ }
-+
-+ if (cal_mc_api) {
-+ struct v4l2_subdev_route *route = NULL;
-+ struct v4l2_subdev_route *r;
-+ struct v4l2_subdev_state *state;
-+
-+ /* Find the PHY connected to this video device */
-+
-+ ctx->phy = cal_camerarx_get_phy_from_entity(remote_pad->entity);
-+
-+ state = v4l2_subdev_lock_and_get_active_state(&ctx->phy->subdev);
-+
-+ /* Find the stream */
-+
-+ for_each_active_route(&state->routing, r) {
-+ if (r->source_pad != remote_pad->index)
-+ continue;
-+
-+ route = r;
-+
-+ break;
-+ }
-+
-+ if (!route) {
-+ v4l2_subdev_unlock_state(state);
-+ ctx_err(ctx, "Failed to find route\n");
-+ ret = -ENODEV;
-+ goto error_release_buffers;
-+ }
-+
-+ ctx->stream = route->sink_stream;
-+
-+ v4l2_subdev_unlock_state(state);
-+ }
+
ret = video_device_pipeline_alloc_start(&ctx->vdev);
if (ret < 0) {
ctx_err(ctx, "Failed to start media pipeline: %d\n", ret);
+ goto error_release_buffers;
+ }
+
++ /* Find the PHY connected to this video device */
++ if (cal_mc_api)
++ ctx->phy = cal_camerarx_get_phy_from_entity(phy_source_pad->entity);
++
+ /*
+ * Verify that the currently configured format matches the output of
+ * the connected CAMERARX.
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
cal_ctx_set_dma_addr(ctx, addr);
cal_ctx_start(ctx);
- ret = v4l2_subdev_call(&ctx->phy->subdev, video, s_stream, 1);
-+ ret = v4l2_subdev_enable_streams(&ctx->phy->subdev, remote_pad->index,
-+ BIT(0));
++ ret = v4l2_subdev_enable_streams(&ctx->phy->subdev,
++ phy_source_pad->index, BIT(0));
if (ret)
goto error_stop;
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_start_streaming(struct
static void cal_stop_streaming(struct vb2_queue *vq)
{
struct cal_ctx *ctx = vb2_get_drv_priv(vq);
-+ struct media_pad *remote_pad;
++ struct media_pad *phy_source_pad;
cal_ctx_stop(ctx);
- v4l2_subdev_call(&ctx->phy->subdev, video, s_stream, 0);
-+ remote_pad = media_pad_remote_pad_first(&ctx->pad);
++ phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
+
-+ v4l2_subdev_disable_streams(&ctx->phy->subdev, remote_pad->index,
++ v4l2_subdev_disable_streams(&ctx->phy->subdev, phy_source_pad->index,
+ BIT(0));
pm_runtime_put_sync(ctx->cal->dev);
@@ drivers/media/platform/ti/cal/cal-video.c: static void cal_stop_streaming(struct
}
static const struct vb2_ops cal_video_qops = {
-@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
+@@ drivers/media/platform/ti/cal/cal-video.c: static const struct v4l2_file_operations cal_fops = {
+
+ static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
+ {
++ struct v4l2_subdev *sd = ctx->phy->source;
+ struct v4l2_mbus_framefmt mbus_fmt;
const struct cal_format_info *fmtinfo;
unsigned int i, j, k;
- int ret = 0;
-+ struct v4l2_subdev *sd = ctx->phy->source;
-
- /* Enumerate sub device formats and enable all matching local formats */
- ctx->active_fmt = devm_kcalloc(ctx->cal->dev, cal_num_formats,
@@ drivers/media/platform/ti/cal/cal-video.c: static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
- memset(&mbus_code, 0, sizeof(mbus_code));
- mbus_code.index = j;
- mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+
- ret = v4l2_subdev_call(ctx->phy->source, pad, enum_mbus_code,
- NULL, &mbus_code);
+ ret = v4l2_subdev_call_state_active(sd, pad, enum_mbus_code,
@@ drivers/media/platform/ti/cal/cal-video.c: int cal_ctx_v4l2_register(struct cal_
+ }
+ } else {
+ ret = media_create_pad_link(&ctx->phy->subdev.entity,
-+ CAL_CAMERARX_PAD_FIRST_SOURCE, &vfd->entity, 0,
-+ MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
++ CAL_CAMERARX_PAD_FIRST_SOURCE,
++ &vfd->entity, 0,
++ MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+ if (ret) {
+ ctx_err(ctx,
+ "Failed to create media link for context %u\n",
@@ drivers/media/platform/ti/cal/cal-video.c: int cal_ctx_v4l2_register(struct cal_
ctx_info(ctx, "V4L2 device registered as %s\n",
## drivers/media/platform/ti/cal/cal.c ##
-@@ drivers/media/platform/ti/cal/cal.c: static bool cal_ctx_wr_dma_stopped(struct cal_ctx *ctx)
- }
-
- static int
--cal_get_remote_frame_desc_entry(struct cal_camerarx *phy,
-+cal_get_remote_frame_desc_entry(struct cal_camerarx *phy, u32 stream,
- struct v4l2_mbus_frame_desc_entry *entry)
- {
- struct v4l2_mbus_frame_desc fd;
-+ unsigned int i;
- int ret;
-
- ret = cal_camerarx_get_remote_frame_desc(phy, &fd);
-@@ drivers/media/platform/ti/cal/cal.c: cal_get_remote_frame_desc_entry(struct cal_camerarx *phy,
- return ret;
- }
-
-- if (fd.num_entries == 0) {
-- dev_err(phy->cal->dev,
-- "No streams found in the remote frame descriptor\n");
--
-- return -ENODEV;
-+ for (i = 0; i < fd.num_entries; i++) {
-+ if (stream == fd.entry[i].stream) {
-+ *entry = fd.entry[i];
-+ return 0;
-+ }
- }
-
-- if (fd.num_entries > 1)
-- dev_dbg(phy->cal->dev,
-- "Multiple streams not supported in remote frame descriptor, using the first one\n");
-+ dev_err(phy->cal->dev,
-+ "Failed to find stream %u from remote frame descriptor\n",
-+ stream);
-
-- *entry = fd.entry[0];
--
-- return 0;
-+ return -ENODEV;
- }
-
- int cal_ctx_prepare(struct cal_ctx *ctx)
@@ drivers/media/platform/ti/cal/cal.c: int cal_ctx_prepare(struct cal_ctx *ctx)
- struct v4l2_mbus_frame_desc_entry entry;
- int ret;
-
-- ret = cal_get_remote_frame_desc_entry(ctx->phy, &entry);
-+ ret = cal_get_remote_frame_desc_entry(ctx->phy, ctx->stream, &entry);
-
- if (ret == -ENOIOCTLCMD) {
ctx->vc = 0;
ctx->datatype = CAL_CSI2_CTX_DT_ANY;
} else if (!ret) {
@@ drivers/media/platform/ti/cal/cal.c: static struct cal_ctx *cal_ctx_create(struc
ctx->dma_ctx = inst;
ctx->csi2_ctx = inst;
ctx->cport = inst;
-+ ctx->stream = 0;
-
- ret = cal_ctx_v4l2_init(ctx);
- if (ret) {
@@ drivers/media/platform/ti/cal/cal.c: static int cal_probe(struct platform_device *pdev)
}
@@ drivers/media/platform/ti/cal/cal.c: static int cal_probe(struct platform_device
- continue;
+ if (!cal_mc_api) {
+ for (i = 0; i < cal->data->num_csi2_phy; ++i) {
++ struct cal_ctx *ctx;
++
+ if (!cal->phy[i]->source_node)
+ continue;
+
-+ cal->ctx[cal->num_contexts] = cal_ctx_create(cal, i);
-+ if (!cal->ctx[cal->num_contexts]) {
++ ctx = cal_ctx_create(cal, i);
++ if (!ctx) {
+ cal_err(cal, "Failed to create context %u\n", cal->num_contexts);
+ ret = -ENODEV;
+ goto error_context;
+ }
+
-+ cal->ctx[cal->num_contexts]->phy = cal->phy[i];
++ ctx->phy = cal->phy[i];
- cal->ctx[cal->num_contexts] = cal_ctx_create(cal, i);
- if (!cal->ctx[cal->num_contexts]) {
- cal_err(cal, "Failed to create context %u\n", cal->num_contexts);
- ret = -ENODEV;
- goto error_context;
-+ cal->num_contexts++;
++ cal->ctx[cal->num_contexts++] = ctx;
}
+ } else {
+ for (i = 0; i < ARRAY_SIZE(cal->ctx); ++i) {
-+ cal->ctx[i] = cal_ctx_create(cal, i);
-+ if (!cal->ctx[i]) {
++ struct cal_ctx *ctx;
++
++ ctx = cal_ctx_create(cal, i);
++ if (!ctx) {
+ cal_err(cal, "Failed to create context %u\n", i);
+ ret = -ENODEV;
+ goto error_context;
+ }
- cal->num_contexts++;
-+ cal->num_contexts++;
++ cal->ctx[cal->num_contexts++] = ctx;
+ }
}
@@ drivers/media/platform/ti/cal/cal.h
#define CAL_CAMERARX_NUM_PADS (1 + CAL_CAMERARX_NUM_SOURCE_PADS)
static inline bool cal_rx_pad_is_sink(u32 pad)
-@@ drivers/media/platform/ti/cal/cal.h: struct cal_ctx {
- u8 pix_proc;
- u8 vc;
- u8 datatype;
-+ u32 stream;
+@@ drivers/media/platform/ti/cal/cal.h: const struct cal_format_info *cal_format_by_code(u32 code);
- bool use_pix_proc;
- };
-@@ drivers/media/platform/ti/cal/cal.h: void cal_quickdump_regs(struct cal_dev *cal);
+ void cal_quickdump_regs(struct cal_dev *cal);
- int cal_camerarx_get_remote_frame_desc(struct cal_camerarx *phy,
- struct v4l2_mbus_frame_desc *desc);
+struct cal_camerarx *cal_camerarx_get_phy_from_entity(struct media_entity *entity);
void cal_camerarx_disable(struct cal_camerarx *phy);
void cal_camerarx_i913_errata(struct cal_camerarx *phy);
base-commit: 83e0f265aa8d0e37cc8e15d318b64da0ec03ff41
prerequisite-patch-id: e800a6da6afee40be8a946ccf63518f6109749dd
prerequisite-patch-id: eb409cc6ffb895128d98b3fa664dcdcafd5e7dfc
prerequisite-patch-id: dedc1c09e4cff1dc58ce909e469bae30a3778a07
prerequisite-patch-id: 1e85d833252748e723b59f90788019fdeca92884
prerequisite-patch-id: bb4f7477e206ed1936e4632e7baa6514f7d957f4
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/4] media: ti: cal: Clean up mbus formats uses
2023-02-28 17:16 [PATCH v2 0/4] media: ti: cal: Streams support Tomi Valkeinen
@ 2023-02-28 17:16 ` Tomi Valkeinen
2023-03-01 9:17 ` Jacopo Mondi
2023-02-28 17:16 ` [PATCH v2 2/4] media: ti: cal: Use subdev state Tomi Valkeinen
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Tomi Valkeinen @ 2023-02-28 17:16 UTC (permalink / raw)
To: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus
Cc: Kieran Bingham, Jai Luthra, Vaishnav Achath, Tomi Valkeinen
Clean up the CAL drivers uses of mbus formats:
- Switch all YUV formats from 2X8 formats to 1X16, as those are what
should be used for CSI-2 bus.
- Drop 24 and 32 bit formats, as the driver doesn't support them (see
cal_ctx_pix_proc_config()).
- Switch RGB565_2X8_LE to RGB565_1X16 (for the same reason as for the
YUV formats) and drop RGB565_2X8_BE as it cannot be supported with
CSI-2.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
drivers/media/platform/ti/cal/cal-camerarx.c | 2 +-
drivers/media/platform/ti/cal/cal-video.c | 2 +-
drivers/media/platform/ti/cal/cal.c | 34 +++-----------------
3 files changed, 7 insertions(+), 31 deletions(-)
diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
index 16ae52879a79..267089b0fea0 100644
--- a/drivers/media/platform/ti/cal/cal-camerarx.c
+++ b/drivers/media/platform/ti/cal/cal-camerarx.c
@@ -817,7 +817,7 @@ static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
.format = {
.width = 640,
.height = 480,
- .code = MEDIA_BUS_FMT_UYVY8_2X8,
+ .code = MEDIA_BUS_FMT_UYVY8_1X16,
.field = V4L2_FIELD_NONE,
.colorspace = V4L2_COLORSPACE_SRGB,
.ycbcr_enc = V4L2_YCBCR_ENC_601,
diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
index ca906a9e4222..ed92e23d4b16 100644
--- a/drivers/media/platform/ti/cal/cal-video.c
+++ b/drivers/media/platform/ti/cal/cal-video.c
@@ -894,7 +894,7 @@ static int cal_ctx_v4l2_init_mc_format(struct cal_ctx *ctx)
const struct cal_format_info *fmtinfo;
struct v4l2_pix_format *pix_fmt = &ctx->v_fmt.fmt.pix;
- fmtinfo = cal_format_by_code(MEDIA_BUS_FMT_UYVY8_2X8);
+ fmtinfo = cal_format_by_code(MEDIA_BUS_FMT_UYVY8_1X16);
if (!fmtinfo)
return -EINVAL;
diff --git a/drivers/media/platform/ti/cal/cal.c b/drivers/media/platform/ti/cal/cal.c
index 1236215ec70e..760c58cb3b3e 100644
--- a/drivers/media/platform/ti/cal/cal.c
+++ b/drivers/media/platform/ti/cal/cal.c
@@ -61,48 +61,24 @@ MODULE_PARM_DESC(mc_api, "activates the MC API");
const struct cal_format_info cal_formats[] = {
{
.fourcc = V4L2_PIX_FMT_YUYV,
- .code = MEDIA_BUS_FMT_YUYV8_2X8,
+ .code = MEDIA_BUS_FMT_YUYV8_1X16,
.bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_UYVY,
- .code = MEDIA_BUS_FMT_UYVY8_2X8,
+ .code = MEDIA_BUS_FMT_UYVY8_1X16,
.bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_YVYU,
- .code = MEDIA_BUS_FMT_YVYU8_2X8,
+ .code = MEDIA_BUS_FMT_YVYU8_1X16,
.bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_VYUY,
- .code = MEDIA_BUS_FMT_VYUY8_2X8,
+ .code = MEDIA_BUS_FMT_VYUY8_1X16,
.bpp = 16,
}, {
.fourcc = V4L2_PIX_FMT_RGB565, /* gggbbbbb rrrrrggg */
- .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
+ .code = MEDIA_BUS_FMT_RGB565_1X16,
.bpp = 16,
- }, {
- .fourcc = V4L2_PIX_FMT_RGB565X, /* rrrrrggg gggbbbbb */
- .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
- .bpp = 16,
- }, {
- .fourcc = V4L2_PIX_FMT_RGB555, /* gggbbbbb arrrrrgg */
- .code = MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE,
- .bpp = 16,
- }, {
- .fourcc = V4L2_PIX_FMT_RGB555X, /* arrrrrgg gggbbbbb */
- .code = MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE,
- .bpp = 16,
- }, {
- .fourcc = V4L2_PIX_FMT_RGB24, /* rgb */
- .code = MEDIA_BUS_FMT_RGB888_2X12_LE,
- .bpp = 24,
- }, {
- .fourcc = V4L2_PIX_FMT_BGR24, /* bgr */
- .code = MEDIA_BUS_FMT_RGB888_2X12_BE,
- .bpp = 24,
- }, {
- .fourcc = V4L2_PIX_FMT_RGB32, /* argb */
- .code = MEDIA_BUS_FMT_ARGB8888_1X32,
- .bpp = 32,
}, {
.fourcc = V4L2_PIX_FMT_SBGGR8,
.code = MEDIA_BUS_FMT_SBGGR8_1X8,
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] media: ti: cal: Use subdev state
2023-02-28 17:16 [PATCH v2 0/4] media: ti: cal: Streams support Tomi Valkeinen
2023-02-28 17:16 ` [PATCH v2 1/4] media: ti: cal: Clean up mbus formats uses Tomi Valkeinen
@ 2023-02-28 17:16 ` Tomi Valkeinen
2023-03-01 9:31 ` Jacopo Mondi
2023-02-28 17:16 ` [PATCH v2 3/4] media: ti: cal: Implement get_frame_desc for camera-rx Tomi Valkeinen
2023-02-28 17:16 ` [PATCH v2 4/4] media: ti: cal: Add multiplexed streams support Tomi Valkeinen
3 siblings, 1 reply; 14+ messages in thread
From: Tomi Valkeinen @ 2023-02-28 17:16 UTC (permalink / raw)
To: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus
Cc: Kieran Bingham, Jai Luthra, Vaishnav Achath, Tomi Valkeinen
Change TI CAL driver to use subdev state. No functional changes
(intended).
This allows us to get rid of the 'formats' field, as the formats are
kept in the state, and also the 'mutex' as we already have state locking.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
drivers/media/platform/ti/cal/cal-camerarx.c | 134 ++++++-------------
drivers/media/platform/ti/cal/cal-video.c | 21 ++-
drivers/media/platform/ti/cal/cal.h | 8 --
3 files changed, 56 insertions(+), 107 deletions(-)
diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
index 267089b0fea0..3dfcb276d367 100644
--- a/drivers/media/platform/ti/cal/cal-camerarx.c
+++ b/drivers/media/platform/ti/cal/cal-camerarx.c
@@ -50,10 +50,16 @@ static s64 cal_camerarx_get_ext_link_freq(struct cal_camerarx *phy)
struct v4l2_mbus_config_mipi_csi2 *mipi_csi2 = &phy->endpoint.bus.mipi_csi2;
u32 num_lanes = mipi_csi2->num_data_lanes;
const struct cal_format_info *fmtinfo;
+ struct v4l2_subdev_state *state;
+ struct v4l2_mbus_framefmt *fmt;
u32 bpp;
s64 freq;
- fmtinfo = cal_format_by_code(phy->formats[CAL_CAMERARX_PAD_SINK].code);
+ state = v4l2_subdev_get_locked_active_state(&phy->subdev);
+
+ fmt = v4l2_subdev_get_pad_format(&phy->subdev, state, CAL_CAMERARX_PAD_SINK);
+
+ fmtinfo = cal_format_by_code(fmt->code);
if (!fmtinfo)
return -EINVAL;
@@ -620,34 +626,20 @@ static inline struct cal_camerarx *to_cal_camerarx(struct v4l2_subdev *sd)
return container_of(sd, struct cal_camerarx, subdev);
}
-static struct v4l2_mbus_framefmt *
-cal_camerarx_get_pad_format(struct cal_camerarx *phy,
- struct v4l2_subdev_state *state,
- unsigned int pad, u32 which)
-{
- switch (which) {
- case V4L2_SUBDEV_FORMAT_TRY:
- return v4l2_subdev_get_try_format(&phy->subdev, state, pad);
- case V4L2_SUBDEV_FORMAT_ACTIVE:
- return &phy->formats[pad];
- default:
- return NULL;
- }
-}
-
static int cal_camerarx_sd_s_stream(struct v4l2_subdev *sd, int enable)
{
struct cal_camerarx *phy = to_cal_camerarx(sd);
+ struct v4l2_subdev_state *state;
int ret = 0;
- mutex_lock(&phy->mutex);
+ state = v4l2_subdev_lock_and_get_active_state(sd);
if (enable)
ret = cal_camerarx_start(phy);
else
cal_camerarx_stop(phy);
- mutex_unlock(&phy->mutex);
+ v4l2_subdev_unlock_state(state);
return ret;
}
@@ -657,62 +649,44 @@ static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_mbus_code_enum *code)
{
struct cal_camerarx *phy = to_cal_camerarx(sd);
- int ret = 0;
-
- mutex_lock(&phy->mutex);
/* No transcoding, source and sink codes must match. */
if (cal_rx_pad_is_source(code->pad)) {
struct v4l2_mbus_framefmt *fmt;
- if (code->index > 0) {
- ret = -EINVAL;
- goto out;
- }
+ if (code->index > 0)
+ return -EINVAL;
- fmt = cal_camerarx_get_pad_format(phy, state,
- CAL_CAMERARX_PAD_SINK,
- code->which);
+ fmt = v4l2_subdev_get_pad_format(&phy->subdev, state,
+ CAL_CAMERARX_PAD_SINK);
code->code = fmt->code;
} else {
- if (code->index >= cal_num_formats) {
- ret = -EINVAL;
- goto out;
- }
+ if (code->index >= cal_num_formats)
+ return -EINVAL;
code->code = cal_formats[code->index].code;
}
-out:
- mutex_unlock(&phy->mutex);
-
- return ret;
+ return 0;
}
static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_frame_size_enum *fse)
{
- struct cal_camerarx *phy = to_cal_camerarx(sd);
const struct cal_format_info *fmtinfo;
- int ret = 0;
if (fse->index > 0)
return -EINVAL;
- mutex_lock(&phy->mutex);
-
/* No transcoding, source and sink formats must match. */
if (cal_rx_pad_is_source(fse->pad)) {
struct v4l2_mbus_framefmt *fmt;
- fmt = cal_camerarx_get_pad_format(phy, state,
- CAL_CAMERARX_PAD_SINK,
- fse->which);
- if (fse->code != fmt->code) {
- ret = -EINVAL;
- goto out;
- }
+ fmt = v4l2_subdev_get_pad_format(sd, state,
+ CAL_CAMERARX_PAD_SINK);
+ if (fse->code != fmt->code)
+ return -EINVAL;
fse->min_width = fmt->width;
fse->max_width = fmt->width;
@@ -720,10 +694,8 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
fse->max_height = fmt->height;
} else {
fmtinfo = cal_format_by_code(fse->code);
- if (!fmtinfo) {
- ret = -EINVAL;
- goto out;
- }
+ if (!fmtinfo)
+ return -EINVAL;
fse->min_width = CAL_MIN_WIDTH_BYTES * 8 / ALIGN(fmtinfo->bpp, 8);
fse->max_width = CAL_MAX_WIDTH_BYTES * 8 / ALIGN(fmtinfo->bpp, 8);
@@ -731,27 +703,6 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
fse->max_height = CAL_MAX_HEIGHT_LINES;
}
-out:
- mutex_unlock(&phy->mutex);
-
- return ret;
-}
-
-static int cal_camerarx_sd_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *state,
- struct v4l2_subdev_format *format)
-{
- struct cal_camerarx *phy = to_cal_camerarx(sd);
- struct v4l2_mbus_framefmt *fmt;
-
- mutex_lock(&phy->mutex);
-
- fmt = cal_camerarx_get_pad_format(phy, state, format->pad,
- format->which);
- format->format = *fmt;
-
- mutex_unlock(&phy->mutex);
-
return 0;
}
@@ -759,14 +710,13 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format)
{
- struct cal_camerarx *phy = to_cal_camerarx(sd);
const struct cal_format_info *fmtinfo;
struct v4l2_mbus_framefmt *fmt;
unsigned int bpp;
/* No transcoding, source and sink formats must match. */
if (cal_rx_pad_is_source(format->pad))
- return cal_camerarx_sd_get_fmt(sd, state, format);
+ return v4l2_subdev_get_fmt(sd, state, format);
/*
* Default to the first format if the requested media bus code isn't
@@ -790,20 +740,13 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
/* Store the format and propagate it to the source pad. */
- mutex_lock(&phy->mutex);
-
- fmt = cal_camerarx_get_pad_format(phy, state,
- CAL_CAMERARX_PAD_SINK,
- format->which);
+ fmt = v4l2_subdev_get_pad_format(sd, state, CAL_CAMERARX_PAD_SINK);
*fmt = format->format;
- fmt = cal_camerarx_get_pad_format(phy, state,
- CAL_CAMERARX_PAD_FIRST_SOURCE,
- format->which);
+ fmt = v4l2_subdev_get_pad_format(sd, state,
+ CAL_CAMERARX_PAD_FIRST_SOURCE);
*fmt = format->format;
- mutex_unlock(&phy->mutex);
-
return 0;
}
@@ -837,7 +780,7 @@ static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
.init_cfg = cal_camerarx_sd_init_cfg,
.enum_mbus_code = cal_camerarx_sd_enum_mbus_code,
.enum_frame_size = cal_camerarx_sd_enum_frame_size,
- .get_fmt = cal_camerarx_sd_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = cal_camerarx_sd_set_fmt,
};
@@ -872,7 +815,6 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
phy->instance = instance;
spin_lock_init(&phy->vc_lock);
- mutex_init(&phy->mutex);
phy->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
(instance == 0) ?
@@ -882,7 +824,7 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
if (IS_ERR(phy->base)) {
cal_err(cal, "failed to ioremap\n");
ret = PTR_ERR(phy->base);
- goto error;
+ goto err_entity_cleanup;
}
cal_dbg(1, cal, "ioresource %s at %pa - %pa\n",
@@ -890,11 +832,11 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
ret = cal_camerarx_regmap_init(cal, phy);
if (ret)
- goto error;
+ goto err_entity_cleanup;
ret = cal_camerarx_parse_dt(phy);
if (ret)
- goto error;
+ goto err_entity_cleanup;
/* Initialize the V4L2 subdev and media entity. */
sd = &phy->subdev;
@@ -911,19 +853,21 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
ret = media_entity_pads_init(&sd->entity, ARRAY_SIZE(phy->pads),
phy->pads);
if (ret)
- goto error;
+ goto err_entity_cleanup;
- ret = cal_camerarx_sd_init_cfg(sd, NULL);
+ ret = v4l2_subdev_init_finalize(sd);
if (ret)
- goto error;
+ goto err_entity_cleanup;
ret = v4l2_device_register_subdev(&cal->v4l2_dev, sd);
if (ret)
- goto error;
+ goto err_free_state;
return phy;
-error:
+err_free_state:
+ v4l2_subdev_cleanup(sd);
+err_entity_cleanup:
media_entity_cleanup(&phy->subdev.entity);
kfree(phy);
return ERR_PTR(ret);
@@ -935,9 +879,9 @@ void cal_camerarx_destroy(struct cal_camerarx *phy)
return;
v4l2_device_unregister_subdev(&phy->subdev);
+ v4l2_subdev_cleanup(&phy->subdev);
media_entity_cleanup(&phy->subdev.entity);
of_node_put(phy->source_ep_node);
of_node_put(phy->source_node);
- mutex_destroy(&phy->mutex);
kfree(phy);
}
diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
index ed92e23d4b16..a8abcd0fee17 100644
--- a/drivers/media/platform/ti/cal/cal-video.c
+++ b/drivers/media/platform/ti/cal/cal-video.c
@@ -687,21 +687,34 @@ static void cal_release_buffers(struct cal_ctx *ctx,
static int cal_video_check_format(struct cal_ctx *ctx)
{
const struct v4l2_mbus_framefmt *format;
+ struct v4l2_subdev_state *state;
struct media_pad *remote_pad;
+ int ret = 0;
remote_pad = media_pad_remote_pad_first(&ctx->pad);
if (!remote_pad)
return -ENODEV;
- format = &ctx->phy->formats[remote_pad->index];
+ state = v4l2_subdev_lock_and_get_active_state(&ctx->phy->subdev);
+
+ format = v4l2_subdev_get_pad_format(&ctx->phy->subdev, state, remote_pad->index);
+ if (!format) {
+ ret = -EINVAL;
+ goto out;
+ }
if (ctx->fmtinfo->code != format->code ||
ctx->v_fmt.fmt.pix.height != format->height ||
ctx->v_fmt.fmt.pix.width != format->width ||
- ctx->v_fmt.fmt.pix.field != format->field)
- return -EPIPE;
+ ctx->v_fmt.fmt.pix.field != format->field) {
+ ret = -EPIPE;
+ goto out;
+ }
- return 0;
+out:
+ v4l2_subdev_unlock_state(state);
+
+ return ret;
}
static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
diff --git a/drivers/media/platform/ti/cal/cal.h b/drivers/media/platform/ti/cal/cal.h
index de73d6d21b6f..55d4736fed18 100644
--- a/drivers/media/platform/ti/cal/cal.h
+++ b/drivers/media/platform/ti/cal/cal.h
@@ -177,7 +177,6 @@ struct cal_camerarx {
struct v4l2_subdev subdev;
struct media_pad pads[CAL_CAMERARX_NUM_PADS];
- struct v4l2_mbus_framefmt formats[CAL_CAMERARX_NUM_PADS];
/* protects the vc_* fields below */
spinlock_t vc_lock;
@@ -185,13 +184,6 @@ struct cal_camerarx {
u16 vc_frame_number[4];
u32 vc_sequence[4];
- /*
- * Lock for camerarx ops. Protects:
- * - formats
- * - enable_count
- */
- struct mutex mutex;
-
unsigned int enable_count;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] media: ti: cal: Implement get_frame_desc for camera-rx
2023-02-28 17:16 [PATCH v2 0/4] media: ti: cal: Streams support Tomi Valkeinen
2023-02-28 17:16 ` [PATCH v2 1/4] media: ti: cal: Clean up mbus formats uses Tomi Valkeinen
2023-02-28 17:16 ` [PATCH v2 2/4] media: ti: cal: Use subdev state Tomi Valkeinen
@ 2023-02-28 17:16 ` Tomi Valkeinen
2023-03-01 9:40 ` Jacopo Mondi
2023-02-28 17:16 ` [PATCH v2 4/4] media: ti: cal: Add multiplexed streams support Tomi Valkeinen
3 siblings, 1 reply; 14+ messages in thread
From: Tomi Valkeinen @ 2023-02-28 17:16 UTC (permalink / raw)
To: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus
Cc: Kieran Bingham, Jai Luthra, Vaishnav Achath, Tomi Valkeinen
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@ideasonboard.com>
---
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");
+
+ fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+ fd->num_entries = 1;
+ fd->entry[0] = remote_desc.entry[0];
+
+ return 0;
+}
+
static const struct v4l2_subdev_video_ops cal_camerarx_video_ops = {
.s_stream = cal_camerarx_sd_s_stream,
};
@@ -782,6 +789,7 @@ static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
.enum_frame_size = cal_camerarx_sd_enum_frame_size,
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = cal_camerarx_sd_set_fmt,
+ .get_frame_desc = cal_camerarx_get_frame_desc,
};
static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
diff --git a/drivers/media/platform/ti/cal/cal.c b/drivers/media/platform/ti/cal/cal.c
index 760c58cb3b3e..bb782193cf65 100644
--- a/drivers/media/platform/ti/cal/cal.c
+++ b/drivers/media/platform/ti/cal/cal.c
@@ -446,30 +446,24 @@ static bool cal_ctx_wr_dma_stopped(struct cal_ctx *ctx)
}
static int
-cal_get_remote_frame_desc_entry(struct cal_camerarx *phy,
+cal_get_remote_frame_desc_entry(struct cal_ctx *ctx,
struct v4l2_mbus_frame_desc_entry *entry)
{
struct v4l2_mbus_frame_desc fd;
+ struct media_pad *phy_source_pad;
int ret;
- ret = cal_camerarx_get_remote_frame_desc(phy, &fd);
- if (ret) {
- if (ret != -ENOIOCTLCMD)
- dev_err(phy->cal->dev,
- "Failed to get remote frame desc: %d\n", ret);
- return ret;
- }
-
- if (fd.num_entries == 0) {
- dev_err(phy->cal->dev,
- "No streams found in the remote frame descriptor\n");
-
+ phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
+ if (!phy_source_pad)
return -ENODEV;
- }
- if (fd.num_entries > 1)
- dev_dbg(phy->cal->dev,
- "Multiple streams not supported in remote frame descriptor, using the first one\n");
+ ret = v4l2_subdev_call(&ctx->phy->subdev, pad, get_frame_desc,
+ phy_source_pad->index, &fd);
+ if (ret)
+ return ret;
+
+ if (fd.num_entries != 1)
+ return -EINVAL;
*entry = fd.entry[0];
@@ -481,7 +475,7 @@ int cal_ctx_prepare(struct cal_ctx *ctx)
struct v4l2_mbus_frame_desc_entry entry;
int ret;
- ret = cal_get_remote_frame_desc_entry(ctx->phy, &entry);
+ ret = cal_get_remote_frame_desc_entry(ctx, &entry);
if (ret == -ENOIOCTLCMD) {
ctx->vc = 0;
diff --git a/drivers/media/platform/ti/cal/cal.h b/drivers/media/platform/ti/cal/cal.h
index 55d4736fed18..0856297adc0b 100644
--- a/drivers/media/platform/ti/cal/cal.h
+++ b/drivers/media/platform/ti/cal/cal.h
@@ -319,8 +319,6 @@ const struct cal_format_info *cal_format_by_code(u32 code);
void cal_quickdump_regs(struct cal_dev *cal);
-int cal_camerarx_get_remote_frame_desc(struct cal_camerarx *phy,
- struct v4l2_mbus_frame_desc *desc);
void cal_camerarx_disable(struct cal_camerarx *phy);
void cal_camerarx_i913_errata(struct cal_camerarx *phy);
struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] media: ti: cal: Add multiplexed streams support
2023-02-28 17:16 [PATCH v2 0/4] media: ti: cal: Streams support Tomi Valkeinen
` (2 preceding siblings ...)
2023-02-28 17:16 ` [PATCH v2 3/4] media: ti: cal: Implement get_frame_desc for camera-rx Tomi Valkeinen
@ 2023-02-28 17:16 ` Tomi Valkeinen
2023-03-01 9:58 ` Jacopo Mondi
3 siblings, 1 reply; 14+ messages in thread
From: Tomi Valkeinen @ 2023-02-28 17:16 UTC (permalink / raw)
To: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus
Cc: Kieran Bingham, Jai Luthra, Vaishnav Achath, Tomi Valkeinen
Add routing and stream_config support to CAL driver.
Add multiplexed streams support. CAL has 8 dma-engines and can capture 8
separate streams at the same time.
Add 8 video device nodes, each representing a single dma-engine, and set
the number of source pads on camerarx to 8. Each video node can be
connected to any of the source pads on either of the camerarx instances
using media links. Camerarx internal routing is used to route the
incoming CSI-2 streams to one of the 8 source pads.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
drivers/media/platform/ti/cal/cal-camerarx.c | 267 ++++++++++++++-----
drivers/media/platform/ti/cal/cal-video.c | 121 ++++++---
drivers/media/platform/ti/cal/cal.c | 43 ++-
drivers/media/platform/ti/cal/cal.h | 3 +-
4 files changed, 330 insertions(+), 104 deletions(-)
diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
index 95e0ad59a39b..8e373c817cdf 100644
--- a/drivers/media/platform/ti/cal/cal-camerarx.c
+++ b/drivers/media/platform/ti/cal/cal-camerarx.c
@@ -49,21 +49,38 @@ static s64 cal_camerarx_get_ext_link_freq(struct cal_camerarx *phy)
{
struct v4l2_mbus_config_mipi_csi2 *mipi_csi2 = &phy->endpoint.bus.mipi_csi2;
u32 num_lanes = mipi_csi2->num_data_lanes;
- const struct cal_format_info *fmtinfo;
struct v4l2_subdev_state *state;
- struct v4l2_mbus_framefmt *fmt;
u32 bpp;
s64 freq;
+ /*
+ * v4l2_get_link_freq() uses V4L2_CID_LINK_FREQ first, and falls back
+ * to V4L2_CID_PIXEL_RATE if V4L2_CID_LINK_FREQ is not available.
+ *
+ * With multistream input there is no single pixel rate, and thus we
+ * cannot use V4L2_CID_PIXEL_RATE, so we pass 0 as the bpp which
+ * causes v4l2_get_link_freq() to return an error if it falls back to
+ * V4L2_CID_PIXEL_RATE.
+ */
+
state = v4l2_subdev_get_locked_active_state(&phy->subdev);
- fmt = v4l2_subdev_get_pad_format(&phy->subdev, state, CAL_CAMERARX_PAD_SINK);
+ if (state->routing.num_routes > 1) {
+ bpp = 0;
+ } else {
+ struct v4l2_subdev_route *route = &state->routing.routes[0];
+ const struct cal_format_info *fmtinfo;
+ struct v4l2_mbus_framefmt *fmt;
- fmtinfo = cal_format_by_code(fmt->code);
- if (!fmtinfo)
- return -EINVAL;
+ fmt = v4l2_subdev_state_get_stream_format(
+ state, route->sink_pad, route->sink_stream);
- bpp = fmtinfo->bpp;
+ fmtinfo = cal_format_by_code(fmt->code);
+ if (!fmtinfo)
+ return -EINVAL;
+
+ bpp = fmtinfo->bpp;
+ }
freq = v4l2_get_link_freq(phy->source->ctrl_handler, bpp, 2 * num_lanes);
if (freq < 0) {
@@ -284,15 +301,32 @@ static void cal_camerarx_ppi_disable(struct cal_camerarx *phy)
0, CAL_CSI2_PPI_CTRL_IF_EN_MASK);
}
-static int cal_camerarx_start(struct cal_camerarx *phy)
+static int cal_camerarx_start(struct cal_camerarx *phy, u32 sink_stream)
{
+ struct media_pad *remote_pad;
s64 link_freq;
u32 sscounter;
u32 val;
int ret;
+ remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
+
+ /*
+ * We need to enable the PHY hardware when enabling the first stream,
+ * but for the following streams we just propagate the enable_streams
+ * to the source.
+ */
+
if (phy->enable_count > 0) {
+ ret = v4l2_subdev_enable_streams(phy->source, remote_pad->index,
+ BIT(sink_stream));
+ if (ret) {
+ phy_err(phy, "enable streams failed in source: %d\n", ret);
+ return ret;
+ }
+
phy->enable_count++;
+
return 0;
}
@@ -394,7 +428,8 @@ static int cal_camerarx_start(struct cal_camerarx *phy)
* Start the source to enable the CSI-2 HS clock. We can now wait for
* CSI-2 PHY reset to complete.
*/
- ret = v4l2_subdev_call(phy->source, video, s_stream, 1);
+ ret = v4l2_subdev_enable_streams(phy->source, remote_pad->index,
+ BIT(sink_stream));
if (ret) {
v4l2_subdev_call(phy->source, core, s_power, 0);
cal_camerarx_disable_irqs(phy);
@@ -425,12 +460,22 @@ static int cal_camerarx_start(struct cal_camerarx *phy)
return 0;
}
-static void cal_camerarx_stop(struct cal_camerarx *phy)
+static void cal_camerarx_stop(struct cal_camerarx *phy, u32 sink_stream)
{
+ struct media_pad *remote_pad;
int ret;
- if (--phy->enable_count > 0)
+ remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
+
+ if (--phy->enable_count > 0) {
+ ret = v4l2_subdev_disable_streams(phy->source,
+ remote_pad->index,
+ BIT(sink_stream));
+ if (ret)
+ phy_err(phy, "stream off failed in subdev\n");
+
return;
+ }
cal_camerarx_ppi_disable(phy);
@@ -450,7 +495,9 @@ static void cal_camerarx_stop(struct cal_camerarx *phy)
/* Disable the phy */
cal_camerarx_disable(phy);
- if (v4l2_subdev_call(phy->source, video, s_stream, 0))
+ ret = v4l2_subdev_disable_streams(phy->source, remote_pad->index,
+ BIT(sink_stream));
+ if (ret)
phy_err(phy, "stream off failed in subdev\n");
ret = v4l2_subdev_call(phy->source, core, s_power, 0);
@@ -599,30 +646,56 @@ static inline struct cal_camerarx *to_cal_camerarx(struct v4l2_subdev *sd)
return container_of(sd, struct cal_camerarx, subdev);
}
-static int cal_camerarx_sd_s_stream(struct v4l2_subdev *sd, int enable)
+struct cal_camerarx *
+cal_camerarx_get_phy_from_entity(struct media_entity *entity)
+{
+ struct v4l2_subdev *sd;
+
+ sd = media_entity_to_v4l2_subdev(entity);
+ if (!sd)
+ return NULL;
+
+ return to_cal_camerarx(sd);
+}
+
+static int cal_camerarx_sd_enable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ u32 pad, u64 streams_mask)
{
struct cal_camerarx *phy = to_cal_camerarx(sd);
- struct v4l2_subdev_state *state;
- int ret = 0;
+ u32 sink_stream;
+ int ret;
- state = v4l2_subdev_lock_and_get_active_state(sd);
+ ret = v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0,
+ NULL, &sink_stream);
+ if (ret)
+ return ret;
- if (enable)
- ret = cal_camerarx_start(phy);
- else
- cal_camerarx_stop(phy);
+ return cal_camerarx_start(phy, sink_stream);
+}
- v4l2_subdev_unlock_state(state);
+static int cal_camerarx_sd_disable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ u32 pad, u64 streams_mask)
+{
+ struct cal_camerarx *phy = to_cal_camerarx(sd);
+ u32 sink_stream;
+ int ret;
- return ret;
+ ret = v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0,
+ NULL, &sink_stream);
+ if (ret)
+ return ret;
+
+ cal_camerarx_stop(phy, sink_stream);
+
+ return 0;
}
static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
{
- struct cal_camerarx *phy = to_cal_camerarx(sd);
-
/* No transcoding, source and sink codes must match. */
if (cal_rx_pad_is_source(code->pad)) {
struct v4l2_mbus_framefmt *fmt;
@@ -630,8 +703,12 @@ static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
if (code->index > 0)
return -EINVAL;
- fmt = v4l2_subdev_get_pad_format(&phy->subdev, state,
- CAL_CAMERARX_PAD_SINK);
+ fmt = v4l2_subdev_state_get_opposite_stream_format(state,
+ code->pad,
+ code->stream);
+ if (!fmt)
+ return -EINVAL;
+
code->code = fmt->code;
} else {
if (code->index >= cal_num_formats)
@@ -656,8 +733,12 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
if (cal_rx_pad_is_source(fse->pad)) {
struct v4l2_mbus_framefmt *fmt;
- fmt = v4l2_subdev_get_pad_format(sd, state,
- CAL_CAMERARX_PAD_SINK);
+ fmt = v4l2_subdev_state_get_opposite_stream_format(state,
+ fse->pad,
+ fse->stream);
+ if (!fmt)
+ return -EINVAL;
+
if (fse->code != fmt->code)
return -EINVAL;
@@ -713,36 +794,78 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
/* Store the format and propagate it to the source pad. */
- fmt = v4l2_subdev_get_pad_format(sd, state, CAL_CAMERARX_PAD_SINK);
+ fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
+ format->stream);
+ if (!fmt)
+ return -EINVAL;
+
*fmt = format->format;
- fmt = v4l2_subdev_get_pad_format(sd, state,
- CAL_CAMERARX_PAD_FIRST_SOURCE);
+ fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
+ format->stream);
+ if (!fmt)
+ return -EINVAL;
+
*fmt = format->format;
return 0;
}
+static int cal_camerarx_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_krouting *routing)
+{
+ static const struct v4l2_mbus_framefmt format = {
+ .width = 640,
+ .height = 480,
+ .code = MEDIA_BUS_FMT_UYVY8_1X16,
+ .field = V4L2_FIELD_NONE,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .ycbcr_enc = V4L2_YCBCR_ENC_601,
+ .quantization = V4L2_QUANTIZATION_LIM_RANGE,
+ .xfer_func = V4L2_XFER_FUNC_SRGB,
+ };
+ int ret;
+
+ ret = v4l2_subdev_routing_validate(sd, routing,
+ V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 |
+ V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING);
+ if (ret)
+ return ret;
+
+ ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int cal_camerarx_sd_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ enum v4l2_subdev_format_whence which,
+ struct v4l2_subdev_krouting *routing)
+{
+ return cal_camerarx_set_routing(sd, state, routing);
+}
+
static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state)
{
- struct v4l2_subdev_format format = {
- .which = state ? V4L2_SUBDEV_FORMAT_TRY
- : V4L2_SUBDEV_FORMAT_ACTIVE,
- .pad = CAL_CAMERARX_PAD_SINK,
- .format = {
- .width = 640,
- .height = 480,
- .code = MEDIA_BUS_FMT_UYVY8_1X16,
- .field = V4L2_FIELD_NONE,
- .colorspace = V4L2_COLORSPACE_SRGB,
- .ycbcr_enc = V4L2_YCBCR_ENC_601,
- .quantization = V4L2_QUANTIZATION_LIM_RANGE,
- .xfer_func = V4L2_XFER_FUNC_SRGB,
- },
+ struct v4l2_subdev_route routes[] = { {
+ .sink_pad = 0,
+ .sink_stream = 0,
+ .source_pad = 1,
+ .source_stream = 0,
+ .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+ } };
+
+ struct v4l2_subdev_krouting routing = {
+ .num_routes = 1,
+ .routes = routes,
};
- return cal_camerarx_sd_set_fmt(sd, state, &format);
+ /* Initialize routing to single route to the fist source pad */
+ return cal_camerarx_set_routing(sd, state, &routing);
}
static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
@@ -751,54 +874,76 @@ static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
struct cal_camerarx *phy = to_cal_camerarx(sd);
struct v4l2_mbus_frame_desc remote_desc;
const struct media_pad *remote_pad;
+ struct v4l2_subdev_state *state;
+ u32 sink_stream;
+ unsigned int i;
int ret;
+ state = v4l2_subdev_lock_and_get_active_state(sd);
+
+ ret = v4l2_subdev_routing_find_opposite_end(&state->routing,
+ pad, 0,
+ NULL, &sink_stream);
+ if (ret)
+ goto out_unlock;
+
remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
- if (!remote_pad)
- return -EPIPE;
+ if (!remote_pad) {
+ ret = -EPIPE;
+ goto out_unlock;
+ }
ret = v4l2_subdev_call(phy->source, pad, get_frame_desc,
remote_pad->index, &remote_desc);
if (ret)
- return ret;
+ goto out_unlock;
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;
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ for (i = 0; i < remote_desc.num_entries; i++) {
+ if (remote_desc.entry[i].stream == sink_stream)
+ break;
}
- if (remote_desc.num_entries > 1)
- dev_dbg(phy->cal->dev,
- "Multiple streams not supported in remote frame descriptor, using the first one\n");
+ if (i == remote_desc.num_entries) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
fd->num_entries = 1;
- fd->entry[0] = remote_desc.entry[0];
+ fd->entry[0] = remote_desc.entry[i];
- return 0;
-}
+out_unlock:
+ v4l2_subdev_unlock_state(state);
-static const struct v4l2_subdev_video_ops cal_camerarx_video_ops = {
- .s_stream = cal_camerarx_sd_s_stream,
-};
+ return ret;
+}
static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
+ .enable_streams = cal_camerarx_sd_enable_streams,
+ .disable_streams = cal_camerarx_sd_disable_streams,
.init_cfg = cal_camerarx_sd_init_cfg,
.enum_mbus_code = cal_camerarx_sd_enum_mbus_code,
.enum_frame_size = cal_camerarx_sd_enum_frame_size,
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = cal_camerarx_sd_set_fmt,
+ .set_routing = cal_camerarx_sd_set_routing,
.get_frame_desc = cal_camerarx_get_frame_desc,
};
static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
- .video = &cal_camerarx_video_ops,
.pad = &cal_camerarx_pad_ops,
};
static struct media_entity_operations cal_camerarx_media_ops = {
.link_validate = v4l2_subdev_link_validate,
+ .has_pad_interdep = v4l2_subdev_has_pad_interdep,
};
/* ------------------------------------------------------------------
@@ -850,7 +995,7 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
sd = &phy->subdev;
v4l2_subdev_init(sd, &cal_camerarx_subdev_ops);
sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
- sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+ sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
snprintf(sd->name, sizeof(sd->name), "CAMERARX%u", instance);
sd->dev = cal->dev;
diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
index a8abcd0fee17..00a79178a8b0 100644
--- a/drivers/media/platform/ti/cal/cal-video.c
+++ b/drivers/media/platform/ti/cal/cal-video.c
@@ -122,9 +122,10 @@ static int __subdev_get_format(struct cal_ctx *ctx,
.pad = 0,
};
struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
+ struct v4l2_subdev *sd = ctx->phy->source;
int ret;
- ret = v4l2_subdev_call(ctx->phy->source, pad, get_fmt, NULL, &sd_fmt);
+ ret = v4l2_subdev_call_state_active(sd, pad, get_fmt, &sd_fmt);
if (ret)
return ret;
@@ -144,11 +145,12 @@ static int __subdev_set_format(struct cal_ctx *ctx,
.pad = 0,
};
struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
+ struct v4l2_subdev *sd = ctx->phy->source;
int ret;
*mbus_fmt = *fmt;
- ret = v4l2_subdev_call(ctx->phy->source, pad, set_fmt, NULL, &sd_fmt);
+ ret = v4l2_subdev_call_state_active(sd, pad, set_fmt, &sd_fmt);
if (ret)
return ret;
@@ -190,6 +192,7 @@ static int cal_legacy_try_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_format *f)
{
struct cal_ctx *ctx = video_drvdata(file);
+ struct v4l2_subdev *sd = ctx->phy->source;
const struct cal_format_info *fmtinfo;
struct v4l2_subdev_frame_size_enum fse = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ -215,8 +218,8 @@ static int cal_legacy_try_fmt_vid_cap(struct file *file, void *priv,
for (fse.index = 0; ; fse.index++) {
int ret;
- ret = v4l2_subdev_call(ctx->phy->source, pad, enum_frame_size,
- NULL, &fse);
+ ret = v4l2_subdev_call_state_active(sd, pad, enum_frame_size,
+ &fse);
if (ret)
break;
@@ -252,6 +255,7 @@ static int cal_legacy_s_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_format *f)
{
struct cal_ctx *ctx = video_drvdata(file);
+ struct v4l2_subdev *sd = &ctx->phy->subdev;
struct vb2_queue *q = &ctx->vb_vidq;
struct v4l2_subdev_format sd_fmt = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ -291,7 +295,7 @@ static int cal_legacy_s_fmt_vid_cap(struct file *file, void *priv,
ctx->v_fmt.fmt.pix.field = sd_fmt.format.field;
cal_calc_format_size(ctx, fmtinfo, &ctx->v_fmt);
- v4l2_subdev_call(&ctx->phy->subdev, pad, set_fmt, NULL, &sd_fmt);
+ v4l2_subdev_call_state_active(sd, pad, set_fmt, &sd_fmt);
ctx->fmtinfo = fmtinfo;
*f = ctx->v_fmt;
@@ -303,6 +307,7 @@ static int cal_legacy_enum_framesizes(struct file *file, void *fh,
struct v4l2_frmsizeenum *fsize)
{
struct cal_ctx *ctx = video_drvdata(file);
+ struct v4l2_subdev *sd = ctx->phy->source;
const struct cal_format_info *fmtinfo;
struct v4l2_subdev_frame_size_enum fse = {
.index = fsize->index,
@@ -321,8 +326,7 @@ static int cal_legacy_enum_framesizes(struct file *file, void *fh,
fse.code = fmtinfo->code;
- ret = v4l2_subdev_call(ctx->phy->source, pad, enum_frame_size, NULL,
- &fse);
+ ret = v4l2_subdev_call_state_active(sd, pad, enum_frame_size, &fse);
if (ret)
return ret;
@@ -364,6 +368,7 @@ static int cal_legacy_enum_frameintervals(struct file *file, void *priv,
struct v4l2_frmivalenum *fival)
{
struct cal_ctx *ctx = video_drvdata(file);
+ struct v4l2_subdev *sd = ctx->phy->source;
const struct cal_format_info *fmtinfo;
struct v4l2_subdev_frame_interval_enum fie = {
.index = fival->index,
@@ -378,8 +383,8 @@ static int cal_legacy_enum_frameintervals(struct file *file, void *priv,
return -EINVAL;
fie.code = fmtinfo->code;
- ret = v4l2_subdev_call(ctx->phy->source, pad, enum_frame_interval,
- NULL, &fie);
+
+ ret = v4l2_subdev_call_state_active(sd, pad, enum_frame_interval, &fie);
if (ret)
return ret;
fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
@@ -688,16 +693,17 @@ static int cal_video_check_format(struct cal_ctx *ctx)
{
const struct v4l2_mbus_framefmt *format;
struct v4l2_subdev_state *state;
- struct media_pad *remote_pad;
+ struct media_pad *phy_source_pad;
int ret = 0;
- remote_pad = media_pad_remote_pad_first(&ctx->pad);
- if (!remote_pad)
+ phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
+ if (!phy_source_pad)
return -ENODEV;
state = v4l2_subdev_lock_and_get_active_state(&ctx->phy->subdev);
- format = v4l2_subdev_get_pad_format(&ctx->phy->subdev, state, remote_pad->index);
+ format = v4l2_subdev_state_get_stream_format(state,
+ phy_source_pad->index, 0);
if (!format) {
ret = -EINVAL;
goto out;
@@ -720,16 +726,28 @@ static int cal_video_check_format(struct cal_ctx *ctx)
static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
{
struct cal_ctx *ctx = vb2_get_drv_priv(vq);
+ struct media_pad *phy_source_pad;
struct cal_buffer *buf;
dma_addr_t addr;
int ret;
+ phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
+ if (!phy_source_pad) {
+ ctx_err(ctx, "Context not connected\n");
+ ret = -ENODEV;
+ goto error_release_buffers;
+ }
+
ret = video_device_pipeline_alloc_start(&ctx->vdev);
if (ret < 0) {
ctx_err(ctx, "Failed to start media pipeline: %d\n", ret);
goto error_release_buffers;
}
+ /* Find the PHY connected to this video device */
+ if (cal_mc_api)
+ ctx->phy = cal_camerarx_get_phy_from_entity(phy_source_pad->entity);
+
/*
* Verify that the currently configured format matches the output of
* the connected CAMERARX.
@@ -762,7 +780,8 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
cal_ctx_set_dma_addr(ctx, addr);
cal_ctx_start(ctx);
- ret = v4l2_subdev_call(&ctx->phy->subdev, video, s_stream, 1);
+ ret = v4l2_subdev_enable_streams(&ctx->phy->subdev,
+ phy_source_pad->index, BIT(0));
if (ret)
goto error_stop;
@@ -787,10 +806,14 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
static void cal_stop_streaming(struct vb2_queue *vq)
{
struct cal_ctx *ctx = vb2_get_drv_priv(vq);
+ struct media_pad *phy_source_pad;
cal_ctx_stop(ctx);
- v4l2_subdev_call(&ctx->phy->subdev, video, s_stream, 0);
+ phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
+
+ v4l2_subdev_disable_streams(&ctx->phy->subdev, phy_source_pad->index,
+ BIT(0));
pm_runtime_put_sync(ctx->cal->dev);
@@ -799,6 +822,9 @@ static void cal_stop_streaming(struct vb2_queue *vq)
cal_release_buffers(ctx, VB2_BUF_STATE_ERROR);
video_device_pipeline_stop(&ctx->vdev);
+
+ if (cal_mc_api)
+ ctx->phy = NULL;
}
static const struct vb2_ops cal_video_qops = {
@@ -827,6 +853,7 @@ static const struct v4l2_file_operations cal_fops = {
static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
{
+ struct v4l2_subdev *sd = ctx->phy->source;
struct v4l2_mbus_framefmt mbus_fmt;
const struct cal_format_info *fmtinfo;
unsigned int i, j, k;
@@ -846,20 +873,20 @@ static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
- ret = v4l2_subdev_call(ctx->phy->source, pad, enum_mbus_code,
- NULL, &mbus_code);
+ ret = v4l2_subdev_call_state_active(sd, pad, enum_mbus_code,
+ &mbus_code);
if (ret == -EINVAL)
break;
if (ret) {
ctx_err(ctx, "Error enumerating mbus codes in subdev %s: %d\n",
- ctx->phy->source->name, ret);
+ sd->name, ret);
return ret;
}
ctx_dbg(2, ctx,
"subdev %s: code: %04x idx: %u\n",
- ctx->phy->source->name, mbus_code.code, j);
+ sd->name, mbus_code.code, j);
for (k = 0; k < cal_num_formats; k++) {
fmtinfo = &cal_formats[k];
@@ -877,7 +904,7 @@ static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
if (i == 0) {
ctx_err(ctx, "No suitable format reported by subdev %s\n",
- ctx->phy->source->name);
+ sd->name);
return -EINVAL;
}
@@ -963,16 +990,50 @@ int cal_ctx_v4l2_register(struct cal_ctx *ctx)
return ret;
}
- ret = media_create_pad_link(&ctx->phy->subdev.entity,
- CAL_CAMERARX_PAD_FIRST_SOURCE,
- &vfd->entity, 0,
- MEDIA_LNK_FL_IMMUTABLE |
- MEDIA_LNK_FL_ENABLED);
- if (ret) {
- ctx_err(ctx, "Failed to create media link for context %u\n",
- ctx->dma_ctx);
- video_unregister_device(vfd);
- return ret;
+ if (cal_mc_api) {
+ u16 phy_idx;
+ u16 pad_idx;
+
+ /* Create links from all video nodes to all PHYs */
+
+ for (phy_idx = 0; phy_idx < ctx->cal->data->num_csi2_phy;
+ ++phy_idx) {
+ for (pad_idx = 1; pad_idx < CAL_CAMERARX_NUM_PADS;
+ ++pad_idx) {
+ /*
+ * Enable only links from video0 to PHY0 pad 1,
+ * and video1 to PHY1 pad 1.
+ */
+ bool enable = (ctx->dma_ctx == 0 &&
+ phy_idx == 0 && pad_idx == 1) ||
+ (ctx->dma_ctx == 1 &&
+ phy_idx == 1 && pad_idx == 1);
+
+ ret = media_create_pad_link(
+ &ctx->cal->phy[phy_idx]->subdev.entity,
+ pad_idx, &vfd->entity, 0,
+ enable ? MEDIA_LNK_FL_ENABLED : 0);
+ if (ret) {
+ ctx_err(ctx,
+ "Failed to create media link for context %u\n",
+ ctx->dma_ctx);
+ video_unregister_device(vfd);
+ return ret;
+ }
+ }
+ }
+ } else {
+ ret = media_create_pad_link(&ctx->phy->subdev.entity,
+ CAL_CAMERARX_PAD_FIRST_SOURCE,
+ &vfd->entity, 0,
+ MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+ if (ret) {
+ ctx_err(ctx,
+ "Failed to create media link for context %u\n",
+ ctx->dma_ctx);
+ video_unregister_device(vfd);
+ return ret;
+ }
}
ctx_info(ctx, "V4L2 device registered as %s\n",
diff --git a/drivers/media/platform/ti/cal/cal.c b/drivers/media/platform/ti/cal/cal.c
index bb782193cf65..983323a109ac 100644
--- a/drivers/media/platform/ti/cal/cal.c
+++ b/drivers/media/platform/ti/cal/cal.c
@@ -481,8 +481,9 @@ int cal_ctx_prepare(struct cal_ctx *ctx)
ctx->vc = 0;
ctx->datatype = CAL_CSI2_CTX_DT_ANY;
} else if (!ret) {
- ctx_dbg(2, ctx, "Framedesc: len %u, vc %u, dt %#x\n",
- entry.length, entry.bus.csi2.vc, entry.bus.csi2.dt);
+ ctx_dbg(2, ctx, "Framedesc: stream %u, len %u, vc %u, dt %#x\n",
+ entry.stream, entry.length, entry.bus.csi2.vc,
+ entry.bus.csi2.dt);
ctx->vc = entry.bus.csi2.vc;
ctx->datatype = entry.bus.csi2.dt;
@@ -1014,7 +1015,6 @@ static struct cal_ctx *cal_ctx_create(struct cal_dev *cal, int inst)
return NULL;
ctx->cal = cal;
- ctx->phy = cal->phy[inst];
ctx->dma_ctx = inst;
ctx->csi2_ctx = inst;
ctx->cport = inst;
@@ -1226,18 +1226,37 @@ static int cal_probe(struct platform_device *pdev)
}
/* Create contexts. */
- for (i = 0; i < cal->data->num_csi2_phy; ++i) {
- if (!cal->phy[i]->source_node)
- continue;
+ if (!cal_mc_api) {
+ for (i = 0; i < cal->data->num_csi2_phy; ++i) {
+ struct cal_ctx *ctx;
+
+ if (!cal->phy[i]->source_node)
+ continue;
+
+ ctx = cal_ctx_create(cal, i);
+ if (!ctx) {
+ cal_err(cal, "Failed to create context %u\n", cal->num_contexts);
+ ret = -ENODEV;
+ goto error_context;
+ }
+
+ ctx->phy = cal->phy[i];
- cal->ctx[cal->num_contexts] = cal_ctx_create(cal, i);
- if (!cal->ctx[cal->num_contexts]) {
- cal_err(cal, "Failed to create context %u\n", cal->num_contexts);
- ret = -ENODEV;
- goto error_context;
+ cal->ctx[cal->num_contexts++] = ctx;
}
+ } else {
+ for (i = 0; i < ARRAY_SIZE(cal->ctx); ++i) {
+ struct cal_ctx *ctx;
+
+ ctx = cal_ctx_create(cal, i);
+ if (!ctx) {
+ cal_err(cal, "Failed to create context %u\n", i);
+ ret = -ENODEV;
+ goto error_context;
+ }
- cal->num_contexts++;
+ cal->ctx[cal->num_contexts++] = ctx;
+ }
}
/* Register the media device. */
diff --git a/drivers/media/platform/ti/cal/cal.h b/drivers/media/platform/ti/cal/cal.h
index 0856297adc0b..44ee0bece56e 100644
--- a/drivers/media/platform/ti/cal/cal.h
+++ b/drivers/media/platform/ti/cal/cal.h
@@ -45,7 +45,7 @@
#define CAL_CAMERARX_PAD_SINK 0
#define CAL_CAMERARX_PAD_FIRST_SOURCE 1
-#define CAL_CAMERARX_NUM_SOURCE_PADS 1
+#define CAL_CAMERARX_NUM_SOURCE_PADS 8
#define CAL_CAMERARX_NUM_PADS (1 + CAL_CAMERARX_NUM_SOURCE_PADS)
static inline bool cal_rx_pad_is_sink(u32 pad)
@@ -319,6 +319,7 @@ const struct cal_format_info *cal_format_by_code(u32 code);
void cal_quickdump_regs(struct cal_dev *cal);
+struct cal_camerarx *cal_camerarx_get_phy_from_entity(struct media_entity *entity);
void cal_camerarx_disable(struct cal_camerarx *phy);
void cal_camerarx_i913_errata(struct cal_camerarx *phy);
struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] media: ti: cal: Clean up mbus formats uses
2023-02-28 17:16 ` [PATCH v2 1/4] media: ti: cal: Clean up mbus formats uses Tomi Valkeinen
@ 2023-03-01 9:17 ` Jacopo Mondi
0 siblings, 0 replies; 14+ messages in thread
From: Jacopo Mondi @ 2023-03-01 9:17 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus,
Kieran Bingham, Jai Luthra, Vaishnav Achath
Hi Tomi
On Tue, Feb 28, 2023 at 07:16:17PM +0200, Tomi Valkeinen wrote:
> Clean up the CAL drivers uses of mbus formats:
>
> - Switch all YUV formats from 2X8 formats to 1X16, as those are what
> should be used for CSI-2 bus.
>
> - Drop 24 and 32 bit formats, as the driver doesn't support them (see
> cal_ctx_pix_proc_config()).
>
> - Switch RGB565_2X8_LE to RGB565_1X16 (for the same reason as for the
> YUV formats) and drop RGB565_2X8_BE as it cannot be supported with
> CSI-2.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> drivers/media/platform/ti/cal/cal-camerarx.c | 2 +-
> drivers/media/platform/ti/cal/cal-video.c | 2 +-
> drivers/media/platform/ti/cal/cal.c | 34 +++-----------------
> 3 files changed, 7 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
> index 16ae52879a79..267089b0fea0 100644
> --- a/drivers/media/platform/ti/cal/cal-camerarx.c
> +++ b/drivers/media/platform/ti/cal/cal-camerarx.c
> @@ -817,7 +817,7 @@ static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
> .format = {
> .width = 640,
> .height = 480,
> - .code = MEDIA_BUS_FMT_UYVY8_2X8,
> + .code = MEDIA_BUS_FMT_UYVY8_1X16,
> .field = V4L2_FIELD_NONE,
> .colorspace = V4L2_COLORSPACE_SRGB,
> .ycbcr_enc = V4L2_YCBCR_ENC_601,
> diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
> index ca906a9e4222..ed92e23d4b16 100644
> --- a/drivers/media/platform/ti/cal/cal-video.c
> +++ b/drivers/media/platform/ti/cal/cal-video.c
> @@ -894,7 +894,7 @@ static int cal_ctx_v4l2_init_mc_format(struct cal_ctx *ctx)
> const struct cal_format_info *fmtinfo;
> struct v4l2_pix_format *pix_fmt = &ctx->v_fmt.fmt.pix;
>
> - fmtinfo = cal_format_by_code(MEDIA_BUS_FMT_UYVY8_2X8);
> + fmtinfo = cal_format_by_code(MEDIA_BUS_FMT_UYVY8_1X16);
> if (!fmtinfo)
> return -EINVAL;
>
> diff --git a/drivers/media/platform/ti/cal/cal.c b/drivers/media/platform/ti/cal/cal.c
> index 1236215ec70e..760c58cb3b3e 100644
> --- a/drivers/media/platform/ti/cal/cal.c
> +++ b/drivers/media/platform/ti/cal/cal.c
> @@ -61,48 +61,24 @@ MODULE_PARM_DESC(mc_api, "activates the MC API");
> const struct cal_format_info cal_formats[] = {
> {
> .fourcc = V4L2_PIX_FMT_YUYV,
> - .code = MEDIA_BUS_FMT_YUYV8_2X8,
> + .code = MEDIA_BUS_FMT_YUYV8_1X16,
> .bpp = 16,
> }, {
> .fourcc = V4L2_PIX_FMT_UYVY,
> - .code = MEDIA_BUS_FMT_UYVY8_2X8,
> + .code = MEDIA_BUS_FMT_UYVY8_1X16,
> .bpp = 16,
> }, {
> .fourcc = V4L2_PIX_FMT_YVYU,
> - .code = MEDIA_BUS_FMT_YVYU8_2X8,
> + .code = MEDIA_BUS_FMT_YVYU8_1X16,
> .bpp = 16,
> }, {
> .fourcc = V4L2_PIX_FMT_VYUY,
> - .code = MEDIA_BUS_FMT_VYUY8_2X8,
> + .code = MEDIA_BUS_FMT_VYUY8_1X16,
> .bpp = 16,
> }, {
> .fourcc = V4L2_PIX_FMT_RGB565, /* gggbbbbb rrrrrggg */
I would drop the comment as it's confusing. As far as I understand it
the RGB565 transmission/reception mode is define uniquely by the CSI-2
specification and the comment here does not match "Figure 112 RGB565
Data Format Reception"
Otherwise
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> - .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
> + .code = MEDIA_BUS_FMT_RGB565_1X16,
> .bpp = 16,
> - }, {
> - .fourcc = V4L2_PIX_FMT_RGB565X, /* rrrrrggg gggbbbbb */
> - .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
> - .bpp = 16,
> - }, {
> - .fourcc = V4L2_PIX_FMT_RGB555, /* gggbbbbb arrrrrgg */
> - .code = MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE,
> - .bpp = 16,
> - }, {
> - .fourcc = V4L2_PIX_FMT_RGB555X, /* arrrrrgg gggbbbbb */
> - .code = MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE,
> - .bpp = 16,
> - }, {
> - .fourcc = V4L2_PIX_FMT_RGB24, /* rgb */
> - .code = MEDIA_BUS_FMT_RGB888_2X12_LE,
> - .bpp = 24,
> - }, {
> - .fourcc = V4L2_PIX_FMT_BGR24, /* bgr */
> - .code = MEDIA_BUS_FMT_RGB888_2X12_BE,
> - .bpp = 24,
> - }, {
> - .fourcc = V4L2_PIX_FMT_RGB32, /* argb */
> - .code = MEDIA_BUS_FMT_ARGB8888_1X32,
> - .bpp = 32,
> }, {
> .fourcc = V4L2_PIX_FMT_SBGGR8,
> .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] media: ti: cal: Use subdev state
2023-02-28 17:16 ` [PATCH v2 2/4] media: ti: cal: Use subdev state Tomi Valkeinen
@ 2023-03-01 9:31 ` Jacopo Mondi
2023-03-01 9:34 ` Tomi Valkeinen
0 siblings, 1 reply; 14+ messages in thread
From: Jacopo Mondi @ 2023-03-01 9:31 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus,
Kieran Bingham, Jai Luthra, Vaishnav Achath
Hi Tomi
On Tue, Feb 28, 2023 at 07:16:18PM +0200, Tomi Valkeinen wrote:
> Change TI CAL driver to use subdev state. No functional changes
> (intended).
>
> This allows us to get rid of the 'formats' field, as the formats are
> kept in the state, and also the 'mutex' as we already have state locking.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> drivers/media/platform/ti/cal/cal-camerarx.c | 134 ++++++-------------
> drivers/media/platform/ti/cal/cal-video.c | 21 ++-
> drivers/media/platform/ti/cal/cal.h | 8 --
> 3 files changed, 56 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
> index 267089b0fea0..3dfcb276d367 100644
> --- a/drivers/media/platform/ti/cal/cal-camerarx.c
> +++ b/drivers/media/platform/ti/cal/cal-camerarx.c
> @@ -50,10 +50,16 @@ static s64 cal_camerarx_get_ext_link_freq(struct cal_camerarx *phy)
> struct v4l2_mbus_config_mipi_csi2 *mipi_csi2 = &phy->endpoint.bus.mipi_csi2;
> u32 num_lanes = mipi_csi2->num_data_lanes;
> const struct cal_format_info *fmtinfo;
> + struct v4l2_subdev_state *state;
> + struct v4l2_mbus_framefmt *fmt;
> u32 bpp;
> s64 freq;
>
> - fmtinfo = cal_format_by_code(phy->formats[CAL_CAMERARX_PAD_SINK].code);
> + state = v4l2_subdev_get_locked_active_state(&phy->subdev);
> +
> + fmt = v4l2_subdev_get_pad_format(&phy->subdev, state, CAL_CAMERARX_PAD_SINK);
> +
> + fmtinfo = cal_format_by_code(fmt->code);
> if (!fmtinfo)
> return -EINVAL;
>
> @@ -620,34 +626,20 @@ static inline struct cal_camerarx *to_cal_camerarx(struct v4l2_subdev *sd)
> return container_of(sd, struct cal_camerarx, subdev);
> }
>
> -static struct v4l2_mbus_framefmt *
> -cal_camerarx_get_pad_format(struct cal_camerarx *phy,
> - struct v4l2_subdev_state *state,
> - unsigned int pad, u32 which)
> -{
> - switch (which) {
> - case V4L2_SUBDEV_FORMAT_TRY:
> - return v4l2_subdev_get_try_format(&phy->subdev, state, pad);
> - case V4L2_SUBDEV_FORMAT_ACTIVE:
> - return &phy->formats[pad];
> - default:
> - return NULL;
> - }
> -}
> -
> static int cal_camerarx_sd_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct cal_camerarx *phy = to_cal_camerarx(sd);
> + struct v4l2_subdev_state *state;
> int ret = 0;
>
> - mutex_lock(&phy->mutex);
> + state = v4l2_subdev_lock_and_get_active_state(sd);
>
> if (enable)
> ret = cal_camerarx_start(phy);
> else
> cal_camerarx_stop(phy);
>
> - mutex_unlock(&phy->mutex);
> + v4l2_subdev_unlock_state(state);
>
> return ret;
> }
> @@ -657,62 +649,44 @@ static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
> struct v4l2_subdev_mbus_code_enum *code)
> {
> struct cal_camerarx *phy = to_cal_camerarx(sd);
> - int ret = 0;
> -
> - mutex_lock(&phy->mutex);
>
> /* No transcoding, source and sink codes must match. */
> if (cal_rx_pad_is_source(code->pad)) {
> struct v4l2_mbus_framefmt *fmt;
>
> - if (code->index > 0) {
> - ret = -EINVAL;
> - goto out;
> - }
> + if (code->index > 0)
> + return -EINVAL;
>
> - fmt = cal_camerarx_get_pad_format(phy, state,
> - CAL_CAMERARX_PAD_SINK,
> - code->which);
> + fmt = v4l2_subdev_get_pad_format(&phy->subdev, state,
> + CAL_CAMERARX_PAD_SINK);
> code->code = fmt->code;
> } else {
> - if (code->index >= cal_num_formats) {
> - ret = -EINVAL;
> - goto out;
> - }
> + if (code->index >= cal_num_formats)
> + return -EINVAL;
>
> code->code = cal_formats[code->index].code;
> }
>
> -out:
> - mutex_unlock(&phy->mutex);
> -
> - return ret;
> + return 0;
> }
>
> static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_frame_size_enum *fse)
> {
> - struct cal_camerarx *phy = to_cal_camerarx(sd);
> const struct cal_format_info *fmtinfo;
> - int ret = 0;
>
> if (fse->index > 0)
> return -EINVAL;
>
> - mutex_lock(&phy->mutex);
> -
> /* No transcoding, source and sink formats must match. */
> if (cal_rx_pad_is_source(fse->pad)) {
> struct v4l2_mbus_framefmt *fmt;
>
> - fmt = cal_camerarx_get_pad_format(phy, state,
> - CAL_CAMERARX_PAD_SINK,
> - fse->which);
> - if (fse->code != fmt->code) {
> - ret = -EINVAL;
> - goto out;
> - }
> + fmt = v4l2_subdev_get_pad_format(sd, state,
> + CAL_CAMERARX_PAD_SINK);
> + if (fse->code != fmt->code)
> + return -EINVAL;
>
> fse->min_width = fmt->width;
> fse->max_width = fmt->width;
> @@ -720,10 +694,8 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
> fse->max_height = fmt->height;
> } else {
> fmtinfo = cal_format_by_code(fse->code);
> - if (!fmtinfo) {
> - ret = -EINVAL;
> - goto out;
> - }
> + if (!fmtinfo)
> + return -EINVAL;
>
> fse->min_width = CAL_MIN_WIDTH_BYTES * 8 / ALIGN(fmtinfo->bpp, 8);
> fse->max_width = CAL_MAX_WIDTH_BYTES * 8 / ALIGN(fmtinfo->bpp, 8);
> @@ -731,27 +703,6 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
> fse->max_height = CAL_MAX_HEIGHT_LINES;
> }
>
> -out:
> - mutex_unlock(&phy->mutex);
> -
> - return ret;
> -}
> -
> -static int cal_camerarx_sd_get_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *state,
> - struct v4l2_subdev_format *format)
> -{
> - struct cal_camerarx *phy = to_cal_camerarx(sd);
> - struct v4l2_mbus_framefmt *fmt;
> -
> - mutex_lock(&phy->mutex);
> -
> - fmt = cal_camerarx_get_pad_format(phy, state, format->pad,
> - format->which);
> - format->format = *fmt;
> -
> - mutex_unlock(&phy->mutex);
> -
> return 0;
> }
>
> @@ -759,14 +710,13 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_format *format)
> {
> - struct cal_camerarx *phy = to_cal_camerarx(sd);
> const struct cal_format_info *fmtinfo;
> struct v4l2_mbus_framefmt *fmt;
> unsigned int bpp;
>
> /* No transcoding, source and sink formats must match. */
> if (cal_rx_pad_is_source(format->pad))
> - return cal_camerarx_sd_get_fmt(sd, state, format);
> + return v4l2_subdev_get_fmt(sd, state, format);
>
> /*
> * Default to the first format if the requested media bus code isn't
> @@ -790,20 +740,13 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
>
> /* Store the format and propagate it to the source pad. */
>
> - mutex_lock(&phy->mutex);
> -
> - fmt = cal_camerarx_get_pad_format(phy, state,
> - CAL_CAMERARX_PAD_SINK,
> - format->which);
> + fmt = v4l2_subdev_get_pad_format(sd, state, CAL_CAMERARX_PAD_SINK);
> *fmt = format->format;
>
> - fmt = cal_camerarx_get_pad_format(phy, state,
> - CAL_CAMERARX_PAD_FIRST_SOURCE,
> - format->which);
> + fmt = v4l2_subdev_get_pad_format(sd, state,
> + CAL_CAMERARX_PAD_FIRST_SOURCE);
> *fmt = format->format;
>
> - mutex_unlock(&phy->mutex);
> -
> return 0;
> }
>
> @@ -837,7 +780,7 @@ static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
> .init_cfg = cal_camerarx_sd_init_cfg,
> .enum_mbus_code = cal_camerarx_sd_enum_mbus_code,
> .enum_frame_size = cal_camerarx_sd_enum_frame_size,
> - .get_fmt = cal_camerarx_sd_get_fmt,
> + .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = cal_camerarx_sd_set_fmt,
> };
>
> @@ -872,7 +815,6 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
> phy->instance = instance;
>
> spin_lock_init(&phy->vc_lock);
> - mutex_init(&phy->mutex);
>
> phy->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> (instance == 0) ?
> @@ -882,7 +824,7 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
> if (IS_ERR(phy->base)) {
> cal_err(cal, "failed to ioremap\n");
> ret = PTR_ERR(phy->base);
> - goto error;
> + goto err_entity_cleanup;
Am I mistaken or media entities are initialized later and you should
here jump to "kfree(phy)" ?
> }
>
> cal_dbg(1, cal, "ioresource %s at %pa - %pa\n",
> @@ -890,11 +832,11 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
>
> ret = cal_camerarx_regmap_init(cal, phy);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
>
> ret = cal_camerarx_parse_dt(phy);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
Same for this two ?
The rest looks good to me!
>
> /* Initialize the V4L2 subdev and media entity. */
> sd = &phy->subdev;
> @@ -911,19 +853,21 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
> ret = media_entity_pads_init(&sd->entity, ARRAY_SIZE(phy->pads),
> phy->pads);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
>
> - ret = cal_camerarx_sd_init_cfg(sd, NULL);
> + ret = v4l2_subdev_init_finalize(sd);
> if (ret)
> - goto error;
> + goto err_entity_cleanup;
>
> ret = v4l2_device_register_subdev(&cal->v4l2_dev, sd);
> if (ret)
> - goto error;
> + goto err_free_state;
>
> return phy;
>
> -error:
> +err_free_state:
> + v4l2_subdev_cleanup(sd);
> +err_entity_cleanup:
> media_entity_cleanup(&phy->subdev.entity);
> kfree(phy);
> return ERR_PTR(ret);
> @@ -935,9 +879,9 @@ void cal_camerarx_destroy(struct cal_camerarx *phy)
> return;
>
> v4l2_device_unregister_subdev(&phy->subdev);
> + v4l2_subdev_cleanup(&phy->subdev);
> media_entity_cleanup(&phy->subdev.entity);
> of_node_put(phy->source_ep_node);
> of_node_put(phy->source_node);
> - mutex_destroy(&phy->mutex);
> kfree(phy);
> }
> diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
> index ed92e23d4b16..a8abcd0fee17 100644
> --- a/drivers/media/platform/ti/cal/cal-video.c
> +++ b/drivers/media/platform/ti/cal/cal-video.c
> @@ -687,21 +687,34 @@ static void cal_release_buffers(struct cal_ctx *ctx,
> static int cal_video_check_format(struct cal_ctx *ctx)
> {
> const struct v4l2_mbus_framefmt *format;
> + struct v4l2_subdev_state *state;
> struct media_pad *remote_pad;
> + int ret = 0;
>
> remote_pad = media_pad_remote_pad_first(&ctx->pad);
> if (!remote_pad)
> return -ENODEV;
>
> - format = &ctx->phy->formats[remote_pad->index];
> + state = v4l2_subdev_lock_and_get_active_state(&ctx->phy->subdev);
> +
> + format = v4l2_subdev_get_pad_format(&ctx->phy->subdev, state, remote_pad->index);
> + if (!format) {
> + ret = -EINVAL;
> + goto out;
> + }
>
> if (ctx->fmtinfo->code != format->code ||
> ctx->v_fmt.fmt.pix.height != format->height ||
> ctx->v_fmt.fmt.pix.width != format->width ||
> - ctx->v_fmt.fmt.pix.field != format->field)
> - return -EPIPE;
> + ctx->v_fmt.fmt.pix.field != format->field) {
> + ret = -EPIPE;
> + goto out;
> + }
>
> - return 0;
> +out:
> + v4l2_subdev_unlock_state(state);
> +
> + return ret;
> }
>
> static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
> diff --git a/drivers/media/platform/ti/cal/cal.h b/drivers/media/platform/ti/cal/cal.h
> index de73d6d21b6f..55d4736fed18 100644
> --- a/drivers/media/platform/ti/cal/cal.h
> +++ b/drivers/media/platform/ti/cal/cal.h
> @@ -177,7 +177,6 @@ struct cal_camerarx {
>
> struct v4l2_subdev subdev;
> struct media_pad pads[CAL_CAMERARX_NUM_PADS];
> - struct v4l2_mbus_framefmt formats[CAL_CAMERARX_NUM_PADS];
>
> /* protects the vc_* fields below */
> spinlock_t vc_lock;
> @@ -185,13 +184,6 @@ struct cal_camerarx {
> u16 vc_frame_number[4];
> u32 vc_sequence[4];
>
> - /*
> - * Lock for camerarx ops. Protects:
> - * - formats
> - * - enable_count
> - */
> - struct mutex mutex;
> -
> unsigned int enable_count;
> };
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] media: ti: cal: Use subdev state
2023-03-01 9:31 ` Jacopo Mondi
@ 2023-03-01 9:34 ` Tomi Valkeinen
2023-03-01 9:38 ` Tomi Valkeinen
0 siblings, 1 reply; 14+ messages in thread
From: Tomi Valkeinen @ 2023-03-01 9:34 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linux-media, Laurent Pinchart, Sakari Ailus, Kieran Bingham,
Jai Luthra, Vaishnav Achath
On 01/03/2023 11:31, Jacopo Mondi wrote:
> Hi Tomi
>
> On Tue, Feb 28, 2023 at 07:16:18PM +0200, Tomi Valkeinen wrote:
>> Change TI CAL driver to use subdev state. No functional changes
>> (intended).
>>
>> This allows us to get rid of the 'formats' field, as the formats are
>> kept in the state, and also the 'mutex' as we already have state locking.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> drivers/media/platform/ti/cal/cal-camerarx.c | 134 ++++++-------------
>> drivers/media/platform/ti/cal/cal-video.c | 21 ++-
>> drivers/media/platform/ti/cal/cal.h | 8 --
>> 3 files changed, 56 insertions(+), 107 deletions(-)
>>
>> diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
>> index 267089b0fea0..3dfcb276d367 100644
>> --- a/drivers/media/platform/ti/cal/cal-camerarx.c
>> +++ b/drivers/media/platform/ti/cal/cal-camerarx.c
>> @@ -50,10 +50,16 @@ static s64 cal_camerarx_get_ext_link_freq(struct cal_camerarx *phy)
>> struct v4l2_mbus_config_mipi_csi2 *mipi_csi2 = &phy->endpoint.bus.mipi_csi2;
>> u32 num_lanes = mipi_csi2->num_data_lanes;
>> const struct cal_format_info *fmtinfo;
>> + struct v4l2_subdev_state *state;
>> + struct v4l2_mbus_framefmt *fmt;
>> u32 bpp;
>> s64 freq;
>>
>> - fmtinfo = cal_format_by_code(phy->formats[CAL_CAMERARX_PAD_SINK].code);
>> + state = v4l2_subdev_get_locked_active_state(&phy->subdev);
>> +
>> + fmt = v4l2_subdev_get_pad_format(&phy->subdev, state, CAL_CAMERARX_PAD_SINK);
>> +
>> + fmtinfo = cal_format_by_code(fmt->code);
>> if (!fmtinfo)
>> return -EINVAL;
>>
>> @@ -620,34 +626,20 @@ static inline struct cal_camerarx *to_cal_camerarx(struct v4l2_subdev *sd)
>> return container_of(sd, struct cal_camerarx, subdev);
>> }
>>
>> -static struct v4l2_mbus_framefmt *
>> -cal_camerarx_get_pad_format(struct cal_camerarx *phy,
>> - struct v4l2_subdev_state *state,
>> - unsigned int pad, u32 which)
>> -{
>> - switch (which) {
>> - case V4L2_SUBDEV_FORMAT_TRY:
>> - return v4l2_subdev_get_try_format(&phy->subdev, state, pad);
>> - case V4L2_SUBDEV_FORMAT_ACTIVE:
>> - return &phy->formats[pad];
>> - default:
>> - return NULL;
>> - }
>> -}
>> -
>> static int cal_camerarx_sd_s_stream(struct v4l2_subdev *sd, int enable)
>> {
>> struct cal_camerarx *phy = to_cal_camerarx(sd);
>> + struct v4l2_subdev_state *state;
>> int ret = 0;
>>
>> - mutex_lock(&phy->mutex);
>> + state = v4l2_subdev_lock_and_get_active_state(sd);
>>
>> if (enable)
>> ret = cal_camerarx_start(phy);
>> else
>> cal_camerarx_stop(phy);
>>
>> - mutex_unlock(&phy->mutex);
>> + v4l2_subdev_unlock_state(state);
>>
>> return ret;
>> }
>> @@ -657,62 +649,44 @@ static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
>> struct v4l2_subdev_mbus_code_enum *code)
>> {
>> struct cal_camerarx *phy = to_cal_camerarx(sd);
>> - int ret = 0;
>> -
>> - mutex_lock(&phy->mutex);
>>
>> /* No transcoding, source and sink codes must match. */
>> if (cal_rx_pad_is_source(code->pad)) {
>> struct v4l2_mbus_framefmt *fmt;
>>
>> - if (code->index > 0) {
>> - ret = -EINVAL;
>> - goto out;
>> - }
>> + if (code->index > 0)
>> + return -EINVAL;
>>
>> - fmt = cal_camerarx_get_pad_format(phy, state,
>> - CAL_CAMERARX_PAD_SINK,
>> - code->which);
>> + fmt = v4l2_subdev_get_pad_format(&phy->subdev, state,
>> + CAL_CAMERARX_PAD_SINK);
>> code->code = fmt->code;
>> } else {
>> - if (code->index >= cal_num_formats) {
>> - ret = -EINVAL;
>> - goto out;
>> - }
>> + if (code->index >= cal_num_formats)
>> + return -EINVAL;
>>
>> code->code = cal_formats[code->index].code;
>> }
>>
>> -out:
>> - mutex_unlock(&phy->mutex);
>> -
>> - return ret;
>> + return 0;
>> }
>>
>> static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
>> struct v4l2_subdev_state *state,
>> struct v4l2_subdev_frame_size_enum *fse)
>> {
>> - struct cal_camerarx *phy = to_cal_camerarx(sd);
>> const struct cal_format_info *fmtinfo;
>> - int ret = 0;
>>
>> if (fse->index > 0)
>> return -EINVAL;
>>
>> - mutex_lock(&phy->mutex);
>> -
>> /* No transcoding, source and sink formats must match. */
>> if (cal_rx_pad_is_source(fse->pad)) {
>> struct v4l2_mbus_framefmt *fmt;
>>
>> - fmt = cal_camerarx_get_pad_format(phy, state,
>> - CAL_CAMERARX_PAD_SINK,
>> - fse->which);
>> - if (fse->code != fmt->code) {
>> - ret = -EINVAL;
>> - goto out;
>> - }
>> + fmt = v4l2_subdev_get_pad_format(sd, state,
>> + CAL_CAMERARX_PAD_SINK);
>> + if (fse->code != fmt->code)
>> + return -EINVAL;
>>
>> fse->min_width = fmt->width;
>> fse->max_width = fmt->width;
>> @@ -720,10 +694,8 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
>> fse->max_height = fmt->height;
>> } else {
>> fmtinfo = cal_format_by_code(fse->code);
>> - if (!fmtinfo) {
>> - ret = -EINVAL;
>> - goto out;
>> - }
>> + if (!fmtinfo)
>> + return -EINVAL;
>>
>> fse->min_width = CAL_MIN_WIDTH_BYTES * 8 / ALIGN(fmtinfo->bpp, 8);
>> fse->max_width = CAL_MAX_WIDTH_BYTES * 8 / ALIGN(fmtinfo->bpp, 8);
>> @@ -731,27 +703,6 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
>> fse->max_height = CAL_MAX_HEIGHT_LINES;
>> }
>>
>> -out:
>> - mutex_unlock(&phy->mutex);
>> -
>> - return ret;
>> -}
>> -
>> -static int cal_camerarx_sd_get_fmt(struct v4l2_subdev *sd,
>> - struct v4l2_subdev_state *state,
>> - struct v4l2_subdev_format *format)
>> -{
>> - struct cal_camerarx *phy = to_cal_camerarx(sd);
>> - struct v4l2_mbus_framefmt *fmt;
>> -
>> - mutex_lock(&phy->mutex);
>> -
>> - fmt = cal_camerarx_get_pad_format(phy, state, format->pad,
>> - format->which);
>> - format->format = *fmt;
>> -
>> - mutex_unlock(&phy->mutex);
>> -
>> return 0;
>> }
>>
>> @@ -759,14 +710,13 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
>> struct v4l2_subdev_state *state,
>> struct v4l2_subdev_format *format)
>> {
>> - struct cal_camerarx *phy = to_cal_camerarx(sd);
>> const struct cal_format_info *fmtinfo;
>> struct v4l2_mbus_framefmt *fmt;
>> unsigned int bpp;
>>
>> /* No transcoding, source and sink formats must match. */
>> if (cal_rx_pad_is_source(format->pad))
>> - return cal_camerarx_sd_get_fmt(sd, state, format);
>> + return v4l2_subdev_get_fmt(sd, state, format);
>>
>> /*
>> * Default to the first format if the requested media bus code isn't
>> @@ -790,20 +740,13 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
>>
>> /* Store the format and propagate it to the source pad. */
>>
>> - mutex_lock(&phy->mutex);
>> -
>> - fmt = cal_camerarx_get_pad_format(phy, state,
>> - CAL_CAMERARX_PAD_SINK,
>> - format->which);
>> + fmt = v4l2_subdev_get_pad_format(sd, state, CAL_CAMERARX_PAD_SINK);
>> *fmt = format->format;
>>
>> - fmt = cal_camerarx_get_pad_format(phy, state,
>> - CAL_CAMERARX_PAD_FIRST_SOURCE,
>> - format->which);
>> + fmt = v4l2_subdev_get_pad_format(sd, state,
>> + CAL_CAMERARX_PAD_FIRST_SOURCE);
>> *fmt = format->format;
>>
>> - mutex_unlock(&phy->mutex);
>> -
>> return 0;
>> }
>>
>> @@ -837,7 +780,7 @@ static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
>> .init_cfg = cal_camerarx_sd_init_cfg,
>> .enum_mbus_code = cal_camerarx_sd_enum_mbus_code,
>> .enum_frame_size = cal_camerarx_sd_enum_frame_size,
>> - .get_fmt = cal_camerarx_sd_get_fmt,
>> + .get_fmt = v4l2_subdev_get_fmt,
>> .set_fmt = cal_camerarx_sd_set_fmt,
>> };
>>
>> @@ -872,7 +815,6 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
>> phy->instance = instance;
>>
>> spin_lock_init(&phy->vc_lock);
>> - mutex_init(&phy->mutex);
>>
>> phy->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> (instance == 0) ?
>> @@ -882,7 +824,7 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
>> if (IS_ERR(phy->base)) {
>> cal_err(cal, "failed to ioremap\n");
>> ret = PTR_ERR(phy->base);
>> - goto error;
>> + goto err_entity_cleanup;
>
> Am I mistaken or media entities are initialized later and you should
> here jump to "kfree(phy)" ?
The media_entity_cleanup doc says:
Calling media_entity_cleanup() on a media_entity whose memory has been
zeroed but that has not been initialized with media_entity_pad_init() is
valid and is a no-op
But probably it looks a bit nicer if we jump to the kfree, as there's
not much downside to that.
Tomi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] media: ti: cal: Use subdev state
2023-03-01 9:34 ` Tomi Valkeinen
@ 2023-03-01 9:38 ` Tomi Valkeinen
0 siblings, 0 replies; 14+ messages in thread
From: Tomi Valkeinen @ 2023-03-01 9:38 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linux-media, Laurent Pinchart, Sakari Ailus, Kieran Bingham,
Jai Luthra, Vaishnav Achath
On 01/03/2023 11:34, Tomi Valkeinen wrote:
> On 01/03/2023 11:31, Jacopo Mondi wrote:
>> Hi Tomi
>>
>> On Tue, Feb 28, 2023 at 07:16:18PM +0200, Tomi Valkeinen wrote:
>>> Change TI CAL driver to use subdev state. No functional changes
>>> (intended).
>>>
>>> This allows us to get rid of the 'formats' field, as the formats are
>>> kept in the state, and also the 'mutex' as we already have state
>>> locking.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> ---
>>> drivers/media/platform/ti/cal/cal-camerarx.c | 134 ++++++-------------
>>> drivers/media/platform/ti/cal/cal-video.c | 21 ++-
>>> drivers/media/platform/ti/cal/cal.h | 8 --
>>> 3 files changed, 56 insertions(+), 107 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c
>>> b/drivers/media/platform/ti/cal/cal-camerarx.c
>>> index 267089b0fea0..3dfcb276d367 100644
>>> --- a/drivers/media/platform/ti/cal/cal-camerarx.c
>>> +++ b/drivers/media/platform/ti/cal/cal-camerarx.c
>>> @@ -50,10 +50,16 @@ static s64 cal_camerarx_get_ext_link_freq(struct
>>> cal_camerarx *phy)
>>> struct v4l2_mbus_config_mipi_csi2 *mipi_csi2 =
>>> &phy->endpoint.bus.mipi_csi2;
>>> u32 num_lanes = mipi_csi2->num_data_lanes;
>>> const struct cal_format_info *fmtinfo;
>>> + struct v4l2_subdev_state *state;
>>> + struct v4l2_mbus_framefmt *fmt;
>>> u32 bpp;
>>> s64 freq;
>>>
>>> - fmtinfo =
>>> cal_format_by_code(phy->formats[CAL_CAMERARX_PAD_SINK].code);
>>> + state = v4l2_subdev_get_locked_active_state(&phy->subdev);
>>> +
>>> + fmt = v4l2_subdev_get_pad_format(&phy->subdev, state,
>>> CAL_CAMERARX_PAD_SINK);
>>> +
>>> + fmtinfo = cal_format_by_code(fmt->code);
>>> if (!fmtinfo)
>>> return -EINVAL;
>>>
>>> @@ -620,34 +626,20 @@ static inline struct cal_camerarx
>>> *to_cal_camerarx(struct v4l2_subdev *sd)
>>> return container_of(sd, struct cal_camerarx, subdev);
>>> }
>>>
>>> -static struct v4l2_mbus_framefmt *
>>> -cal_camerarx_get_pad_format(struct cal_camerarx *phy,
>>> - struct v4l2_subdev_state *state,
>>> - unsigned int pad, u32 which)
>>> -{
>>> - switch (which) {
>>> - case V4L2_SUBDEV_FORMAT_TRY:
>>> - return v4l2_subdev_get_try_format(&phy->subdev, state, pad);
>>> - case V4L2_SUBDEV_FORMAT_ACTIVE:
>>> - return &phy->formats[pad];
>>> - default:
>>> - return NULL;
>>> - }
>>> -}
>>> -
>>> static int cal_camerarx_sd_s_stream(struct v4l2_subdev *sd, int
>>> enable)
>>> {
>>> struct cal_camerarx *phy = to_cal_camerarx(sd);
>>> + struct v4l2_subdev_state *state;
>>> int ret = 0;
>>>
>>> - mutex_lock(&phy->mutex);
>>> + state = v4l2_subdev_lock_and_get_active_state(sd);
>>>
>>> if (enable)
>>> ret = cal_camerarx_start(phy);
>>> else
>>> cal_camerarx_stop(phy);
>>>
>>> - mutex_unlock(&phy->mutex);
>>> + v4l2_subdev_unlock_state(state);
>>>
>>> return ret;
>>> }
>>> @@ -657,62 +649,44 @@ static int
>>> cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
>>> struct v4l2_subdev_mbus_code_enum *code)
>>> {
>>> struct cal_camerarx *phy = to_cal_camerarx(sd);
>>> - int ret = 0;
>>> -
>>> - mutex_lock(&phy->mutex);
>>>
>>> /* No transcoding, source and sink codes must match. */
>>> if (cal_rx_pad_is_source(code->pad)) {
>>> struct v4l2_mbus_framefmt *fmt;
>>>
>>> - if (code->index > 0) {
>>> - ret = -EINVAL;
>>> - goto out;
>>> - }
>>> + if (code->index > 0)
>>> + return -EINVAL;
>>>
>>> - fmt = cal_camerarx_get_pad_format(phy, state,
>>> - CAL_CAMERARX_PAD_SINK,
>>> - code->which);
>>> + fmt = v4l2_subdev_get_pad_format(&phy->subdev, state,
>>> + CAL_CAMERARX_PAD_SINK);
>>> code->code = fmt->code;
>>> } else {
>>> - if (code->index >= cal_num_formats) {
>>> - ret = -EINVAL;
>>> - goto out;
>>> - }
>>> + if (code->index >= cal_num_formats)
>>> + return -EINVAL;
>>>
>>> code->code = cal_formats[code->index].code;
>>> }
>>>
>>> -out:
>>> - mutex_unlock(&phy->mutex);
>>> -
>>> - return ret;
>>> + return 0;
>>> }
>>>
>>> static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
>>> struct v4l2_subdev_state *state,
>>> struct v4l2_subdev_frame_size_enum *fse)
>>> {
>>> - struct cal_camerarx *phy = to_cal_camerarx(sd);
>>> const struct cal_format_info *fmtinfo;
>>> - int ret = 0;
>>>
>>> if (fse->index > 0)
>>> return -EINVAL;
>>>
>>> - mutex_lock(&phy->mutex);
>>> -
>>> /* No transcoding, source and sink formats must match. */
>>> if (cal_rx_pad_is_source(fse->pad)) {
>>> struct v4l2_mbus_framefmt *fmt;
>>>
>>> - fmt = cal_camerarx_get_pad_format(phy, state,
>>> - CAL_CAMERARX_PAD_SINK,
>>> - fse->which);
>>> - if (fse->code != fmt->code) {
>>> - ret = -EINVAL;
>>> - goto out;
>>> - }
>>> + fmt = v4l2_subdev_get_pad_format(sd, state,
>>> + CAL_CAMERARX_PAD_SINK);
>>> + if (fse->code != fmt->code)
>>> + return -EINVAL;
>>>
>>> fse->min_width = fmt->width;
>>> fse->max_width = fmt->width;
>>> @@ -720,10 +694,8 @@ static int
>>> cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
>>> fse->max_height = fmt->height;
>>> } else {
>>> fmtinfo = cal_format_by_code(fse->code);
>>> - if (!fmtinfo) {
>>> - ret = -EINVAL;
>>> - goto out;
>>> - }
>>> + if (!fmtinfo)
>>> + return -EINVAL;
>>>
>>> fse->min_width = CAL_MIN_WIDTH_BYTES * 8 /
>>> ALIGN(fmtinfo->bpp, 8);
>>> fse->max_width = CAL_MAX_WIDTH_BYTES * 8 /
>>> ALIGN(fmtinfo->bpp, 8);
>>> @@ -731,27 +703,6 @@ static int
>>> cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
>>> fse->max_height = CAL_MAX_HEIGHT_LINES;
>>> }
>>>
>>> -out:
>>> - mutex_unlock(&phy->mutex);
>>> -
>>> - return ret;
>>> -}
>>> -
>>> -static int cal_camerarx_sd_get_fmt(struct v4l2_subdev *sd,
>>> - struct v4l2_subdev_state *state,
>>> - struct v4l2_subdev_format *format)
>>> -{
>>> - struct cal_camerarx *phy = to_cal_camerarx(sd);
>>> - struct v4l2_mbus_framefmt *fmt;
>>> -
>>> - mutex_lock(&phy->mutex);
>>> -
>>> - fmt = cal_camerarx_get_pad_format(phy, state, format->pad,
>>> - format->which);
>>> - format->format = *fmt;
>>> -
>>> - mutex_unlock(&phy->mutex);
>>> -
>>> return 0;
>>> }
>>>
>>> @@ -759,14 +710,13 @@ static int cal_camerarx_sd_set_fmt(struct
>>> v4l2_subdev *sd,
>>> struct v4l2_subdev_state *state,
>>> struct v4l2_subdev_format *format)
>>> {
>>> - struct cal_camerarx *phy = to_cal_camerarx(sd);
>>> const struct cal_format_info *fmtinfo;
>>> struct v4l2_mbus_framefmt *fmt;
>>> unsigned int bpp;
>>>
>>> /* No transcoding, source and sink formats must match. */
>>> if (cal_rx_pad_is_source(format->pad))
>>> - return cal_camerarx_sd_get_fmt(sd, state, format);
>>> + return v4l2_subdev_get_fmt(sd, state, format);
>>>
>>> /*
>>> * Default to the first format if the requested media bus code
>>> isn't
>>> @@ -790,20 +740,13 @@ static int cal_camerarx_sd_set_fmt(struct
>>> v4l2_subdev *sd,
>>>
>>> /* Store the format and propagate it to the source pad. */
>>>
>>> - mutex_lock(&phy->mutex);
>>> -
>>> - fmt = cal_camerarx_get_pad_format(phy, state,
>>> - CAL_CAMERARX_PAD_SINK,
>>> - format->which);
>>> + fmt = v4l2_subdev_get_pad_format(sd, state, CAL_CAMERARX_PAD_SINK);
>>> *fmt = format->format;
>>>
>>> - fmt = cal_camerarx_get_pad_format(phy, state,
>>> - CAL_CAMERARX_PAD_FIRST_SOURCE,
>>> - format->which);
>>> + fmt = v4l2_subdev_get_pad_format(sd, state,
>>> + CAL_CAMERARX_PAD_FIRST_SOURCE);
>>> *fmt = format->format;
>>>
>>> - mutex_unlock(&phy->mutex);
>>> -
>>> return 0;
>>> }
>>>
>>> @@ -837,7 +780,7 @@ static const struct v4l2_subdev_pad_ops
>>> cal_camerarx_pad_ops = {
>>> .init_cfg = cal_camerarx_sd_init_cfg,
>>> .enum_mbus_code = cal_camerarx_sd_enum_mbus_code,
>>> .enum_frame_size = cal_camerarx_sd_enum_frame_size,
>>> - .get_fmt = cal_camerarx_sd_get_fmt,
>>> + .get_fmt = v4l2_subdev_get_fmt,
>>> .set_fmt = cal_camerarx_sd_set_fmt,
>>> };
>>>
>>> @@ -872,7 +815,6 @@ struct cal_camerarx *cal_camerarx_create(struct
>>> cal_dev *cal,
>>> phy->instance = instance;
>>>
>>> spin_lock_init(&phy->vc_lock);
>>> - mutex_init(&phy->mutex);
>>>
>>> phy->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>>> (instance == 0) ?
>>> @@ -882,7 +824,7 @@ struct cal_camerarx *cal_camerarx_create(struct
>>> cal_dev *cal,
>>> if (IS_ERR(phy->base)) {
>>> cal_err(cal, "failed to ioremap\n");
>>> ret = PTR_ERR(phy->base);
>>> - goto error;
>>> + goto err_entity_cleanup;
>>
>> Am I mistaken or media entities are initialized later and you should
>> here jump to "kfree(phy)" ?
>
> The media_entity_cleanup doc says:
>
> Calling media_entity_cleanup() on a media_entity whose memory has been
> zeroed but that has not been initialized with media_entity_pad_init() is
> valid and is a no-op
>
> But probably it looks a bit nicer if we jump to the kfree, as there's
> not much downside to that.
Actually, we can just use devm_kzalloc() to get the phy, simplifying
this further.
Tomi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/4] media: ti: cal: Implement get_frame_desc for camera-rx
2023-02-28 17:16 ` [PATCH v2 3/4] media: ti: cal: Implement get_frame_desc for camera-rx Tomi Valkeinen
@ 2023-03-01 9:40 ` Jacopo Mondi
2023-03-01 10:05 ` Tomi Valkeinen
0 siblings, 1 reply; 14+ messages in thread
From: Jacopo Mondi @ 2023-03-01 9:40 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus,
Kieran Bingham, Jai Luthra, Vaishnav Achath
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@ideasonboard.com>
> ---
> 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 ??
As this mostly serves to transport to CAL the DT and VC, I guess it's
fine
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> + fd->num_entries = 1;
> + fd->entry[0] = remote_desc.entry[0];
> +
> + return 0;
> +}
> +
> static const struct v4l2_subdev_video_ops cal_camerarx_video_ops = {
> .s_stream = cal_camerarx_sd_s_stream,
> };
> @@ -782,6 +789,7 @@ static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
> .enum_frame_size = cal_camerarx_sd_enum_frame_size,
> .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = cal_camerarx_sd_set_fmt,
> + .get_frame_desc = cal_camerarx_get_frame_desc,
> };
>
> static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
> diff --git a/drivers/media/platform/ti/cal/cal.c b/drivers/media/platform/ti/cal/cal.c
> index 760c58cb3b3e..bb782193cf65 100644
> --- a/drivers/media/platform/ti/cal/cal.c
> +++ b/drivers/media/platform/ti/cal/cal.c
> @@ -446,30 +446,24 @@ static bool cal_ctx_wr_dma_stopped(struct cal_ctx *ctx)
> }
>
> static int
> -cal_get_remote_frame_desc_entry(struct cal_camerarx *phy,
> +cal_get_remote_frame_desc_entry(struct cal_ctx *ctx,
> struct v4l2_mbus_frame_desc_entry *entry)
> {
> struct v4l2_mbus_frame_desc fd;
> + struct media_pad *phy_source_pad;
> int ret;
>
> - ret = cal_camerarx_get_remote_frame_desc(phy, &fd);
> - if (ret) {
> - if (ret != -ENOIOCTLCMD)
> - dev_err(phy->cal->dev,
> - "Failed to get remote frame desc: %d\n", ret);
> - return ret;
> - }
> -
> - if (fd.num_entries == 0) {
> - dev_err(phy->cal->dev,
> - "No streams found in the remote frame descriptor\n");
> -
> + phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
> + if (!phy_source_pad)
> return -ENODEV;
> - }
>
> - if (fd.num_entries > 1)
> - dev_dbg(phy->cal->dev,
> - "Multiple streams not supported in remote frame descriptor, using the first one\n");
> + ret = v4l2_subdev_call(&ctx->phy->subdev, pad, get_frame_desc,
> + phy_source_pad->index, &fd);
> + if (ret)
> + return ret;
> +
> + if (fd.num_entries != 1)
> + return -EINVAL;
>
> *entry = fd.entry[0];
>
> @@ -481,7 +475,7 @@ int cal_ctx_prepare(struct cal_ctx *ctx)
> struct v4l2_mbus_frame_desc_entry entry;
> int ret;
>
> - ret = cal_get_remote_frame_desc_entry(ctx->phy, &entry);
> + ret = cal_get_remote_frame_desc_entry(ctx, &entry);
>
> if (ret == -ENOIOCTLCMD) {
> ctx->vc = 0;
> diff --git a/drivers/media/platform/ti/cal/cal.h b/drivers/media/platform/ti/cal/cal.h
> index 55d4736fed18..0856297adc0b 100644
> --- a/drivers/media/platform/ti/cal/cal.h
> +++ b/drivers/media/platform/ti/cal/cal.h
> @@ -319,8 +319,6 @@ const struct cal_format_info *cal_format_by_code(u32 code);
>
> void cal_quickdump_regs(struct cal_dev *cal);
>
> -int cal_camerarx_get_remote_frame_desc(struct cal_camerarx *phy,
> - struct v4l2_mbus_frame_desc *desc);
> void cal_camerarx_disable(struct cal_camerarx *phy);
> void cal_camerarx_i913_errata(struct cal_camerarx *phy);
> struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] media: ti: cal: Add multiplexed streams support
2023-02-28 17:16 ` [PATCH v2 4/4] media: ti: cal: Add multiplexed streams support Tomi Valkeinen
@ 2023-03-01 9:58 ` Jacopo Mondi
2023-03-01 10:10 ` Tomi Valkeinen
0 siblings, 1 reply; 14+ messages in thread
From: Jacopo Mondi @ 2023-03-01 9:58 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus,
Kieran Bingham, Jai Luthra, Vaishnav Achath
Hi Tomi
On Tue, Feb 28, 2023 at 07:16:20PM +0200, Tomi Valkeinen wrote:
> Add routing and stream_config support to CAL driver.
>
> Add multiplexed streams support. CAL has 8 dma-engines and can capture 8
> separate streams at the same time.
>
> Add 8 video device nodes, each representing a single dma-engine, and set
> the number of source pads on camerarx to 8. Each video node can be
> connected to any of the source pads on either of the camerarx instances
> using media links. Camerarx internal routing is used to route the
> incoming CSI-2 streams to one of the 8 source pads.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> drivers/media/platform/ti/cal/cal-camerarx.c | 267 ++++++++++++++-----
> drivers/media/platform/ti/cal/cal-video.c | 121 ++++++---
> drivers/media/platform/ti/cal/cal.c | 43 ++-
> drivers/media/platform/ti/cal/cal.h | 3 +-
> 4 files changed, 330 insertions(+), 104 deletions(-)
>
> diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
> index 95e0ad59a39b..8e373c817cdf 100644
> --- a/drivers/media/platform/ti/cal/cal-camerarx.c
> +++ b/drivers/media/platform/ti/cal/cal-camerarx.c
> @@ -49,21 +49,38 @@ static s64 cal_camerarx_get_ext_link_freq(struct cal_camerarx *phy)
> {
> struct v4l2_mbus_config_mipi_csi2 *mipi_csi2 = &phy->endpoint.bus.mipi_csi2;
> u32 num_lanes = mipi_csi2->num_data_lanes;
> - const struct cal_format_info *fmtinfo;
> struct v4l2_subdev_state *state;
> - struct v4l2_mbus_framefmt *fmt;
> u32 bpp;
> s64 freq;
>
> + /*
> + * v4l2_get_link_freq() uses V4L2_CID_LINK_FREQ first, and falls back
> + * to V4L2_CID_PIXEL_RATE if V4L2_CID_LINK_FREQ is not available.
> + *
> + * With multistream input there is no single pixel rate, and thus we
> + * cannot use V4L2_CID_PIXEL_RATE, so we pass 0 as the bpp which
> + * causes v4l2_get_link_freq() to return an error if it falls back to
> + * V4L2_CID_PIXEL_RATE.
> + */
> +
> state = v4l2_subdev_get_locked_active_state(&phy->subdev);
>
> - fmt = v4l2_subdev_get_pad_format(&phy->subdev, state, CAL_CAMERARX_PAD_SINK);
> + if (state->routing.num_routes > 1) {
> + bpp = 0;
> + } else {
> + struct v4l2_subdev_route *route = &state->routing.routes[0];
> + const struct cal_format_info *fmtinfo;
> + struct v4l2_mbus_framefmt *fmt;
>
> - fmtinfo = cal_format_by_code(fmt->code);
> - if (!fmtinfo)
> - return -EINVAL;
> + fmt = v4l2_subdev_state_get_stream_format(
> + state, route->sink_pad, route->sink_stream);
>
> - bpp = fmtinfo->bpp;
> + fmtinfo = cal_format_by_code(fmt->code);
> + if (!fmtinfo)
> + return -EINVAL;
> +
> + bpp = fmtinfo->bpp;
> + }
>
> freq = v4l2_get_link_freq(phy->source->ctrl_handler, bpp, 2 * num_lanes);
> if (freq < 0) {
> @@ -284,15 +301,32 @@ static void cal_camerarx_ppi_disable(struct cal_camerarx *phy)
> 0, CAL_CSI2_PPI_CTRL_IF_EN_MASK);
> }
>
> -static int cal_camerarx_start(struct cal_camerarx *phy)
> +static int cal_camerarx_start(struct cal_camerarx *phy, u32 sink_stream)
> {
> + struct media_pad *remote_pad;
> s64 link_freq;
> u32 sscounter;
> u32 val;
> int ret;
>
> + remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
> +
> + /*
> + * We need to enable the PHY hardware when enabling the first stream,
> + * but for the following streams we just propagate the enable_streams
> + * to the source.
> + */
> +
> if (phy->enable_count > 0) {
> + ret = v4l2_subdev_enable_streams(phy->source, remote_pad->index,
> + BIT(sink_stream));
> + if (ret) {
> + phy_err(phy, "enable streams failed in source: %d\n", ret);
> + return ret;
> + }
> +
> phy->enable_count++;
> +
> return 0;
> }
>
> @@ -394,7 +428,8 @@ static int cal_camerarx_start(struct cal_camerarx *phy)
> * Start the source to enable the CSI-2 HS clock. We can now wait for
> * CSI-2 PHY reset to complete.
> */
> - ret = v4l2_subdev_call(phy->source, video, s_stream, 1);
> + ret = v4l2_subdev_enable_streams(phy->source, remote_pad->index,
> + BIT(sink_stream));
> if (ret) {
> v4l2_subdev_call(phy->source, core, s_power, 0);
> cal_camerarx_disable_irqs(phy);
> @@ -425,12 +460,22 @@ static int cal_camerarx_start(struct cal_camerarx *phy)
> return 0;
> }
>
> -static void cal_camerarx_stop(struct cal_camerarx *phy)
> +static void cal_camerarx_stop(struct cal_camerarx *phy, u32 sink_stream)
> {
> + struct media_pad *remote_pad;
> int ret;
>
> - if (--phy->enable_count > 0)
> + remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
> +
> + if (--phy->enable_count > 0) {
> + ret = v4l2_subdev_disable_streams(phy->source,
> + remote_pad->index,
> + BIT(sink_stream));
> + if (ret)
> + phy_err(phy, "stream off failed in subdev\n");
> +
> return;
> + }
>
> cal_camerarx_ppi_disable(phy);
>
> @@ -450,7 +495,9 @@ static void cal_camerarx_stop(struct cal_camerarx *phy)
> /* Disable the phy */
> cal_camerarx_disable(phy);
>
> - if (v4l2_subdev_call(phy->source, video, s_stream, 0))
> + ret = v4l2_subdev_disable_streams(phy->source, remote_pad->index,
> + BIT(sink_stream));
> + if (ret)
> phy_err(phy, "stream off failed in subdev\n");
>
> ret = v4l2_subdev_call(phy->source, core, s_power, 0);
> @@ -599,30 +646,56 @@ static inline struct cal_camerarx *to_cal_camerarx(struct v4l2_subdev *sd)
> return container_of(sd, struct cal_camerarx, subdev);
> }
>
> -static int cal_camerarx_sd_s_stream(struct v4l2_subdev *sd, int enable)
> +struct cal_camerarx *
> +cal_camerarx_get_phy_from_entity(struct media_entity *entity)
> +{
> + struct v4l2_subdev *sd;
> +
> + sd = media_entity_to_v4l2_subdev(entity);
> + if (!sd)
> + return NULL;
> +
> + return to_cal_camerarx(sd);
> +}
> +
> +static int cal_camerarx_sd_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> {
> struct cal_camerarx *phy = to_cal_camerarx(sd);
> - struct v4l2_subdev_state *state;
> - int ret = 0;
> + u32 sink_stream;
> + int ret;
>
> - state = v4l2_subdev_lock_and_get_active_state(sd);
> + ret = v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0,
> + NULL, &sink_stream);
> + if (ret)
> + return ret;
>
> - if (enable)
> - ret = cal_camerarx_start(phy);
> - else
> - cal_camerarx_stop(phy);
> + return cal_camerarx_start(phy, sink_stream);
> +}
>
> - v4l2_subdev_unlock_state(state);
> +static int cal_camerarx_sd_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> +{
> + struct cal_camerarx *phy = to_cal_camerarx(sd);
> + u32 sink_stream;
> + int ret;
>
> - return ret;
> + ret = v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0,
> + NULL, &sink_stream);
> + if (ret)
> + return ret;
> +
> + cal_camerarx_stop(phy, sink_stream);
> +
> + return 0;
> }
>
> static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_mbus_code_enum *code)
> {
> - struct cal_camerarx *phy = to_cal_camerarx(sd);
> -
> /* No transcoding, source and sink codes must match. */
> if (cal_rx_pad_is_source(code->pad)) {
> struct v4l2_mbus_framefmt *fmt;
> @@ -630,8 +703,12 @@ static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
> if (code->index > 0)
> return -EINVAL;
>
> - fmt = v4l2_subdev_get_pad_format(&phy->subdev, state,
> - CAL_CAMERARX_PAD_SINK);
> + fmt = v4l2_subdev_state_get_opposite_stream_format(state,
> + code->pad,
> + code->stream);
> + if (!fmt)
> + return -EINVAL;
> +
> code->code = fmt->code;
> } else {
> if (code->index >= cal_num_formats)
> @@ -656,8 +733,12 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
> if (cal_rx_pad_is_source(fse->pad)) {
> struct v4l2_mbus_framefmt *fmt;
>
> - fmt = v4l2_subdev_get_pad_format(sd, state,
> - CAL_CAMERARX_PAD_SINK);
> + fmt = v4l2_subdev_state_get_opposite_stream_format(state,
> + fse->pad,
> + fse->stream);
> + if (!fmt)
> + return -EINVAL;
> +
> if (fse->code != fmt->code)
> return -EINVAL;
>
> @@ -713,36 +794,78 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
>
> /* Store the format and propagate it to the source pad. */
>
> - fmt = v4l2_subdev_get_pad_format(sd, state, CAL_CAMERARX_PAD_SINK);
> + fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
> + format->stream);
> + if (!fmt)
> + return -EINVAL;
> +
> *fmt = format->format;
>
> - fmt = v4l2_subdev_get_pad_format(sd, state,
> - CAL_CAMERARX_PAD_FIRST_SOURCE);
> + fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
> + format->stream);
> + if (!fmt)
> + return -EINVAL;
> +
> *fmt = format->format;
>
> return 0;
> }
>
> +static int cal_camerarx_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_krouting *routing)
> +{
> + static const struct v4l2_mbus_framefmt format = {
> + .width = 640,
> + .height = 480,
> + .code = MEDIA_BUS_FMT_UYVY8_1X16,
> + .field = V4L2_FIELD_NONE,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .ycbcr_enc = V4L2_YCBCR_ENC_601,
> + .quantization = V4L2_QUANTIZATION_LIM_RANGE,
> + .xfer_func = V4L2_XFER_FUNC_SRGB,
> + };
> + int ret;
> +
> + ret = v4l2_subdev_routing_validate(sd, routing,
> + V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 |
> + V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING);
> + if (ret)
> + return ret;
> +
> + ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int cal_camerarx_sd_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + enum v4l2_subdev_format_whence which,
> + struct v4l2_subdev_krouting *routing)
> +{
> + return cal_camerarx_set_routing(sd, state, routing);
> +}
> +
> static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state)
> {
> - struct v4l2_subdev_format format = {
> - .which = state ? V4L2_SUBDEV_FORMAT_TRY
> - : V4L2_SUBDEV_FORMAT_ACTIVE,
> - .pad = CAL_CAMERARX_PAD_SINK,
> - .format = {
> - .width = 640,
> - .height = 480,
> - .code = MEDIA_BUS_FMT_UYVY8_1X16,
> - .field = V4L2_FIELD_NONE,
> - .colorspace = V4L2_COLORSPACE_SRGB,
> - .ycbcr_enc = V4L2_YCBCR_ENC_601,
> - .quantization = V4L2_QUANTIZATION_LIM_RANGE,
> - .xfer_func = V4L2_XFER_FUNC_SRGB,
> - },
> + struct v4l2_subdev_route routes[] = { {
> + .sink_pad = 0,
> + .sink_stream = 0,
> + .source_pad = 1,
> + .source_stream = 0,
> + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> + } };
> +
> + struct v4l2_subdev_krouting routing = {
> + .num_routes = 1,
> + .routes = routes,
> };
>
> - return cal_camerarx_sd_set_fmt(sd, state, &format);
> + /* Initialize routing to single route to the fist source pad */
> + return cal_camerarx_set_routing(sd, state, &routing);
> }
>
> static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> @@ -751,54 +874,76 @@ static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> struct cal_camerarx *phy = to_cal_camerarx(sd);
> struct v4l2_mbus_frame_desc remote_desc;
> const struct media_pad *remote_pad;
> + struct v4l2_subdev_state *state;
> + u32 sink_stream;
> + unsigned int i;
> int ret;
>
> + state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> + ret = v4l2_subdev_routing_find_opposite_end(&state->routing,
> + pad, 0,
> + NULL, &sink_stream);
> + if (ret)
> + goto out_unlock;
> +
> remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
> - if (!remote_pad)
> - return -EPIPE;
> + if (!remote_pad) {
> + ret = -EPIPE;
> + goto out_unlock;
> + }
>
> ret = v4l2_subdev_call(phy->source, pad, get_frame_desc,
> remote_pad->index, &remote_desc);
> if (ret)
> - return ret;
> + goto out_unlock;
>
> if (remote_desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
> dev_err(phy->cal->dev,
> "Frame descriptor does not describe CSI-2 link");
I'm all for using standard debug functions, but the rest of the driver
uses cal_err etc. As long as this is intentional, I'm fine with it
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + for (i = 0; i < remote_desc.num_entries; i++) {
> + if (remote_desc.entry[i].stream == sink_stream)
> + break;
> }
>
> - if (remote_desc.num_entries > 1)
> - dev_dbg(phy->cal->dev,
> - "Multiple streams not supported in remote frame descriptor, using the first one\n");
> + if (i == remote_desc.num_entries) {
I would have kept an error message here, or do you think this cannot
happen, or is it possible that a wrong implementation on the
transmitter's get_frame_desc could trigger it ?
All nits, to my understanding the rest looks good
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Thanks
j
> + ret = -EINVAL;
> + goto out_unlock;
> + }
>
> fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> fd->num_entries = 1;
> - fd->entry[0] = remote_desc.entry[0];
> + fd->entry[0] = remote_desc.entry[i];
>
> - return 0;
> -}
> +out_unlock:
> + v4l2_subdev_unlock_state(state);
>
> -static const struct v4l2_subdev_video_ops cal_camerarx_video_ops = {
> - .s_stream = cal_camerarx_sd_s_stream,
> -};
> + return ret;
> +}
>
> static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
> + .enable_streams = cal_camerarx_sd_enable_streams,
> + .disable_streams = cal_camerarx_sd_disable_streams,
> .init_cfg = cal_camerarx_sd_init_cfg,
> .enum_mbus_code = cal_camerarx_sd_enum_mbus_code,
> .enum_frame_size = cal_camerarx_sd_enum_frame_size,
> .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = cal_camerarx_sd_set_fmt,
> + .set_routing = cal_camerarx_sd_set_routing,
> .get_frame_desc = cal_camerarx_get_frame_desc,
> };
>
> static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
> - .video = &cal_camerarx_video_ops,
> .pad = &cal_camerarx_pad_ops,
> };
>
> static struct media_entity_operations cal_camerarx_media_ops = {
> .link_validate = v4l2_subdev_link_validate,
> + .has_pad_interdep = v4l2_subdev_has_pad_interdep,
> };
>
> /* ------------------------------------------------------------------
> @@ -850,7 +995,7 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
> sd = &phy->subdev;
> v4l2_subdev_init(sd, &cal_camerarx_subdev_ops);
> sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> - sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> + sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_STREAMS;
> snprintf(sd->name, sizeof(sd->name), "CAMERARX%u", instance);
> sd->dev = cal->dev;
>
> diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
> index a8abcd0fee17..00a79178a8b0 100644
> --- a/drivers/media/platform/ti/cal/cal-video.c
> +++ b/drivers/media/platform/ti/cal/cal-video.c
> @@ -122,9 +122,10 @@ static int __subdev_get_format(struct cal_ctx *ctx,
> .pad = 0,
> };
> struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
> + struct v4l2_subdev *sd = ctx->phy->source;
> int ret;
>
> - ret = v4l2_subdev_call(ctx->phy->source, pad, get_fmt, NULL, &sd_fmt);
> + ret = v4l2_subdev_call_state_active(sd, pad, get_fmt, &sd_fmt);
> if (ret)
> return ret;
>
> @@ -144,11 +145,12 @@ static int __subdev_set_format(struct cal_ctx *ctx,
> .pad = 0,
> };
> struct v4l2_mbus_framefmt *mbus_fmt = &sd_fmt.format;
> + struct v4l2_subdev *sd = ctx->phy->source;
> int ret;
>
> *mbus_fmt = *fmt;
>
> - ret = v4l2_subdev_call(ctx->phy->source, pad, set_fmt, NULL, &sd_fmt);
> + ret = v4l2_subdev_call_state_active(sd, pad, set_fmt, &sd_fmt);
> if (ret)
> return ret;
>
> @@ -190,6 +192,7 @@ static int cal_legacy_try_fmt_vid_cap(struct file *file, void *priv,
> struct v4l2_format *f)
> {
> struct cal_ctx *ctx = video_drvdata(file);
> + struct v4l2_subdev *sd = ctx->phy->source;
> const struct cal_format_info *fmtinfo;
> struct v4l2_subdev_frame_size_enum fse = {
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> @@ -215,8 +218,8 @@ static int cal_legacy_try_fmt_vid_cap(struct file *file, void *priv,
> for (fse.index = 0; ; fse.index++) {
> int ret;
>
> - ret = v4l2_subdev_call(ctx->phy->source, pad, enum_frame_size,
> - NULL, &fse);
> + ret = v4l2_subdev_call_state_active(sd, pad, enum_frame_size,
> + &fse);
> if (ret)
> break;
>
> @@ -252,6 +255,7 @@ static int cal_legacy_s_fmt_vid_cap(struct file *file, void *priv,
> struct v4l2_format *f)
> {
> struct cal_ctx *ctx = video_drvdata(file);
> + struct v4l2_subdev *sd = &ctx->phy->subdev;
> struct vb2_queue *q = &ctx->vb_vidq;
> struct v4l2_subdev_format sd_fmt = {
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> @@ -291,7 +295,7 @@ static int cal_legacy_s_fmt_vid_cap(struct file *file, void *priv,
> ctx->v_fmt.fmt.pix.field = sd_fmt.format.field;
> cal_calc_format_size(ctx, fmtinfo, &ctx->v_fmt);
>
> - v4l2_subdev_call(&ctx->phy->subdev, pad, set_fmt, NULL, &sd_fmt);
> + v4l2_subdev_call_state_active(sd, pad, set_fmt, &sd_fmt);
>
> ctx->fmtinfo = fmtinfo;
> *f = ctx->v_fmt;
> @@ -303,6 +307,7 @@ static int cal_legacy_enum_framesizes(struct file *file, void *fh,
> struct v4l2_frmsizeenum *fsize)
> {
> struct cal_ctx *ctx = video_drvdata(file);
> + struct v4l2_subdev *sd = ctx->phy->source;
> const struct cal_format_info *fmtinfo;
> struct v4l2_subdev_frame_size_enum fse = {
> .index = fsize->index,
> @@ -321,8 +326,7 @@ static int cal_legacy_enum_framesizes(struct file *file, void *fh,
>
> fse.code = fmtinfo->code;
>
> - ret = v4l2_subdev_call(ctx->phy->source, pad, enum_frame_size, NULL,
> - &fse);
> + ret = v4l2_subdev_call_state_active(sd, pad, enum_frame_size, &fse);
> if (ret)
> return ret;
>
> @@ -364,6 +368,7 @@ static int cal_legacy_enum_frameintervals(struct file *file, void *priv,
> struct v4l2_frmivalenum *fival)
> {
> struct cal_ctx *ctx = video_drvdata(file);
> + struct v4l2_subdev *sd = ctx->phy->source;
> const struct cal_format_info *fmtinfo;
> struct v4l2_subdev_frame_interval_enum fie = {
> .index = fival->index,
> @@ -378,8 +383,8 @@ static int cal_legacy_enum_frameintervals(struct file *file, void *priv,
> return -EINVAL;
>
> fie.code = fmtinfo->code;
> - ret = v4l2_subdev_call(ctx->phy->source, pad, enum_frame_interval,
> - NULL, &fie);
> +
> + ret = v4l2_subdev_call_state_active(sd, pad, enum_frame_interval, &fie);
> if (ret)
> return ret;
> fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> @@ -688,16 +693,17 @@ static int cal_video_check_format(struct cal_ctx *ctx)
> {
> const struct v4l2_mbus_framefmt *format;
> struct v4l2_subdev_state *state;
> - struct media_pad *remote_pad;
> + struct media_pad *phy_source_pad;
> int ret = 0;
>
> - remote_pad = media_pad_remote_pad_first(&ctx->pad);
> - if (!remote_pad)
> + phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
> + if (!phy_source_pad)
> return -ENODEV;
>
> state = v4l2_subdev_lock_and_get_active_state(&ctx->phy->subdev);
>
> - format = v4l2_subdev_get_pad_format(&ctx->phy->subdev, state, remote_pad->index);
> + format = v4l2_subdev_state_get_stream_format(state,
> + phy_source_pad->index, 0);
> if (!format) {
> ret = -EINVAL;
> goto out;
> @@ -720,16 +726,28 @@ static int cal_video_check_format(struct cal_ctx *ctx)
> static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
> {
> struct cal_ctx *ctx = vb2_get_drv_priv(vq);
> + struct media_pad *phy_source_pad;
> struct cal_buffer *buf;
> dma_addr_t addr;
> int ret;
>
> + phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
> + if (!phy_source_pad) {
> + ctx_err(ctx, "Context not connected\n");
> + ret = -ENODEV;
> + goto error_release_buffers;
> + }
> +
> ret = video_device_pipeline_alloc_start(&ctx->vdev);
> if (ret < 0) {
> ctx_err(ctx, "Failed to start media pipeline: %d\n", ret);
> goto error_release_buffers;
> }
>
> + /* Find the PHY connected to this video device */
> + if (cal_mc_api)
> + ctx->phy = cal_camerarx_get_phy_from_entity(phy_source_pad->entity);
> +
> /*
> * Verify that the currently configured format matches the output of
> * the connected CAMERARX.
> @@ -762,7 +780,8 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
> cal_ctx_set_dma_addr(ctx, addr);
> cal_ctx_start(ctx);
>
> - ret = v4l2_subdev_call(&ctx->phy->subdev, video, s_stream, 1);
> + ret = v4l2_subdev_enable_streams(&ctx->phy->subdev,
> + phy_source_pad->index, BIT(0));
> if (ret)
> goto error_stop;
>
> @@ -787,10 +806,14 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
> static void cal_stop_streaming(struct vb2_queue *vq)
> {
> struct cal_ctx *ctx = vb2_get_drv_priv(vq);
> + struct media_pad *phy_source_pad;
>
> cal_ctx_stop(ctx);
>
> - v4l2_subdev_call(&ctx->phy->subdev, video, s_stream, 0);
> + phy_source_pad = media_pad_remote_pad_first(&ctx->pad);
> +
> + v4l2_subdev_disable_streams(&ctx->phy->subdev, phy_source_pad->index,
> + BIT(0));
>
> pm_runtime_put_sync(ctx->cal->dev);
>
> @@ -799,6 +822,9 @@ static void cal_stop_streaming(struct vb2_queue *vq)
> cal_release_buffers(ctx, VB2_BUF_STATE_ERROR);
>
> video_device_pipeline_stop(&ctx->vdev);
> +
> + if (cal_mc_api)
> + ctx->phy = NULL;
> }
>
> static const struct vb2_ops cal_video_qops = {
> @@ -827,6 +853,7 @@ static const struct v4l2_file_operations cal_fops = {
>
> static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
> {
> + struct v4l2_subdev *sd = ctx->phy->source;
> struct v4l2_mbus_framefmt mbus_fmt;
> const struct cal_format_info *fmtinfo;
> unsigned int i, j, k;
> @@ -846,20 +873,20 @@ static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> };
>
> - ret = v4l2_subdev_call(ctx->phy->source, pad, enum_mbus_code,
> - NULL, &mbus_code);
> + ret = v4l2_subdev_call_state_active(sd, pad, enum_mbus_code,
> + &mbus_code);
> if (ret == -EINVAL)
> break;
>
> if (ret) {
> ctx_err(ctx, "Error enumerating mbus codes in subdev %s: %d\n",
> - ctx->phy->source->name, ret);
> + sd->name, ret);
> return ret;
> }
>
> ctx_dbg(2, ctx,
> "subdev %s: code: %04x idx: %u\n",
> - ctx->phy->source->name, mbus_code.code, j);
> + sd->name, mbus_code.code, j);
>
> for (k = 0; k < cal_num_formats; k++) {
> fmtinfo = &cal_formats[k];
> @@ -877,7 +904,7 @@ static int cal_ctx_v4l2_init_formats(struct cal_ctx *ctx)
>
> if (i == 0) {
> ctx_err(ctx, "No suitable format reported by subdev %s\n",
> - ctx->phy->source->name);
> + sd->name);
> return -EINVAL;
> }
>
> @@ -963,16 +990,50 @@ int cal_ctx_v4l2_register(struct cal_ctx *ctx)
> return ret;
> }
>
> - ret = media_create_pad_link(&ctx->phy->subdev.entity,
> - CAL_CAMERARX_PAD_FIRST_SOURCE,
> - &vfd->entity, 0,
> - MEDIA_LNK_FL_IMMUTABLE |
> - MEDIA_LNK_FL_ENABLED);
> - if (ret) {
> - ctx_err(ctx, "Failed to create media link for context %u\n",
> - ctx->dma_ctx);
> - video_unregister_device(vfd);
> - return ret;
> + if (cal_mc_api) {
> + u16 phy_idx;
> + u16 pad_idx;
> +
> + /* Create links from all video nodes to all PHYs */
> +
> + for (phy_idx = 0; phy_idx < ctx->cal->data->num_csi2_phy;
> + ++phy_idx) {
> + for (pad_idx = 1; pad_idx < CAL_CAMERARX_NUM_PADS;
> + ++pad_idx) {
> + /*
> + * Enable only links from video0 to PHY0 pad 1,
> + * and video1 to PHY1 pad 1.
> + */
> + bool enable = (ctx->dma_ctx == 0 &&
> + phy_idx == 0 && pad_idx == 1) ||
> + (ctx->dma_ctx == 1 &&
> + phy_idx == 1 && pad_idx == 1);
> +
> + ret = media_create_pad_link(
> + &ctx->cal->phy[phy_idx]->subdev.entity,
> + pad_idx, &vfd->entity, 0,
> + enable ? MEDIA_LNK_FL_ENABLED : 0);
> + if (ret) {
> + ctx_err(ctx,
> + "Failed to create media link for context %u\n",
> + ctx->dma_ctx);
> + video_unregister_device(vfd);
> + return ret;
> + }
> + }
> + }
> + } else {
> + ret = media_create_pad_link(&ctx->phy->subdev.entity,
> + CAL_CAMERARX_PAD_FIRST_SOURCE,
> + &vfd->entity, 0,
> + MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
> + if (ret) {
> + ctx_err(ctx,
> + "Failed to create media link for context %u\n",
> + ctx->dma_ctx);
> + video_unregister_device(vfd);
> + return ret;
> + }
> }
>
> ctx_info(ctx, "V4L2 device registered as %s\n",
> diff --git a/drivers/media/platform/ti/cal/cal.c b/drivers/media/platform/ti/cal/cal.c
> index bb782193cf65..983323a109ac 100644
> --- a/drivers/media/platform/ti/cal/cal.c
> +++ b/drivers/media/platform/ti/cal/cal.c
> @@ -481,8 +481,9 @@ int cal_ctx_prepare(struct cal_ctx *ctx)
> ctx->vc = 0;
> ctx->datatype = CAL_CSI2_CTX_DT_ANY;
> } else if (!ret) {
> - ctx_dbg(2, ctx, "Framedesc: len %u, vc %u, dt %#x\n",
> - entry.length, entry.bus.csi2.vc, entry.bus.csi2.dt);
> + ctx_dbg(2, ctx, "Framedesc: stream %u, len %u, vc %u, dt %#x\n",
> + entry.stream, entry.length, entry.bus.csi2.vc,
> + entry.bus.csi2.dt);
>
> ctx->vc = entry.bus.csi2.vc;
> ctx->datatype = entry.bus.csi2.dt;
> @@ -1014,7 +1015,6 @@ static struct cal_ctx *cal_ctx_create(struct cal_dev *cal, int inst)
> return NULL;
>
> ctx->cal = cal;
> - ctx->phy = cal->phy[inst];
> ctx->dma_ctx = inst;
> ctx->csi2_ctx = inst;
> ctx->cport = inst;
> @@ -1226,18 +1226,37 @@ static int cal_probe(struct platform_device *pdev)
> }
>
> /* Create contexts. */
> - for (i = 0; i < cal->data->num_csi2_phy; ++i) {
> - if (!cal->phy[i]->source_node)
> - continue;
> + if (!cal_mc_api) {
> + for (i = 0; i < cal->data->num_csi2_phy; ++i) {
> + struct cal_ctx *ctx;
> +
> + if (!cal->phy[i]->source_node)
> + continue;
> +
> + ctx = cal_ctx_create(cal, i);
> + if (!ctx) {
> + cal_err(cal, "Failed to create context %u\n", cal->num_contexts);
> + ret = -ENODEV;
> + goto error_context;
> + }
> +
> + ctx->phy = cal->phy[i];
>
> - cal->ctx[cal->num_contexts] = cal_ctx_create(cal, i);
> - if (!cal->ctx[cal->num_contexts]) {
> - cal_err(cal, "Failed to create context %u\n", cal->num_contexts);
> - ret = -ENODEV;
> - goto error_context;
> + cal->ctx[cal->num_contexts++] = ctx;
> }
> + } else {
> + for (i = 0; i < ARRAY_SIZE(cal->ctx); ++i) {
> + struct cal_ctx *ctx;
> +
> + ctx = cal_ctx_create(cal, i);
> + if (!ctx) {
> + cal_err(cal, "Failed to create context %u\n", i);
> + ret = -ENODEV;
> + goto error_context;
> + }
>
> - cal->num_contexts++;
> + cal->ctx[cal->num_contexts++] = ctx;
> + }
> }
>
> /* Register the media device. */
> diff --git a/drivers/media/platform/ti/cal/cal.h b/drivers/media/platform/ti/cal/cal.h
> index 0856297adc0b..44ee0bece56e 100644
> --- a/drivers/media/platform/ti/cal/cal.h
> +++ b/drivers/media/platform/ti/cal/cal.h
> @@ -45,7 +45,7 @@
>
> #define CAL_CAMERARX_PAD_SINK 0
> #define CAL_CAMERARX_PAD_FIRST_SOURCE 1
> -#define CAL_CAMERARX_NUM_SOURCE_PADS 1
> +#define CAL_CAMERARX_NUM_SOURCE_PADS 8
> #define CAL_CAMERARX_NUM_PADS (1 + CAL_CAMERARX_NUM_SOURCE_PADS)
>
> static inline bool cal_rx_pad_is_sink(u32 pad)
> @@ -319,6 +319,7 @@ const struct cal_format_info *cal_format_by_code(u32 code);
>
> void cal_quickdump_regs(struct cal_dev *cal);
>
> +struct cal_camerarx *cal_camerarx_get_phy_from_entity(struct media_entity *entity);
> void cal_camerarx_disable(struct cal_camerarx *phy);
> void cal_camerarx_i913_errata(struct cal_camerarx *phy);
> struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/4] media: ti: cal: Implement get_frame_desc for camera-rx
2023-03-01 9:40 ` Jacopo Mondi
@ 2023-03-01 10:05 ` Tomi Valkeinen
2023-03-01 10:51 ` Jacopo Mondi
0 siblings, 1 reply; 14+ messages in thread
From: Tomi Valkeinen @ 2023-03-01 10:05 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linux-media, Laurent Pinchart, Sakari Ailus, Kieran Bingham,
Jai Luthra, Vaishnav Achath
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@ideasonboard.com>
>> ---
>> 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@ideasonboard.com>
Thanks!
Tomi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] media: ti: cal: Add multiplexed streams support
2023-03-01 9:58 ` Jacopo Mondi
@ 2023-03-01 10:10 ` Tomi Valkeinen
0 siblings, 0 replies; 14+ messages in thread
From: Tomi Valkeinen @ 2023-03-01 10:10 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linux-media, Laurent Pinchart, Sakari Ailus, Kieran Bingham,
Jai Luthra, Vaishnav Achath
On 01/03/2023 11:58, Jacopo Mondi wrote:
> Hi Tomi
>
> On Tue, Feb 28, 2023 at 07:16:20PM +0200, Tomi Valkeinen wrote:
>> Add routing and stream_config support to CAL driver.
>>
>> Add multiplexed streams support. CAL has 8 dma-engines and can capture 8
>> separate streams at the same time.
>>
>> Add 8 video device nodes, each representing a single dma-engine, and set
>> the number of source pads on camerarx to 8. Each video node can be
>> connected to any of the source pads on either of the camerarx instances
>> using media links. Camerarx internal routing is used to route the
>> incoming CSI-2 streams to one of the 8 source pads.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> drivers/media/platform/ti/cal/cal-camerarx.c | 267 ++++++++++++++-----
>> drivers/media/platform/ti/cal/cal-video.c | 121 ++++++---
>> drivers/media/platform/ti/cal/cal.c | 43 ++-
>> drivers/media/platform/ti/cal/cal.h | 3 +-
>> 4 files changed, 330 insertions(+), 104 deletions(-)
>>
>> diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
>> index 95e0ad59a39b..8e373c817cdf 100644
>> --- a/drivers/media/platform/ti/cal/cal-camerarx.c
>> +++ b/drivers/media/platform/ti/cal/cal-camerarx.c
>> @@ -49,21 +49,38 @@ static s64 cal_camerarx_get_ext_link_freq(struct cal_camerarx *phy)
>> {
>> struct v4l2_mbus_config_mipi_csi2 *mipi_csi2 = &phy->endpoint.bus.mipi_csi2;
>> u32 num_lanes = mipi_csi2->num_data_lanes;
>> - const struct cal_format_info *fmtinfo;
>> struct v4l2_subdev_state *state;
>> - struct v4l2_mbus_framefmt *fmt;
>> u32 bpp;
>> s64 freq;
>>
>> + /*
>> + * v4l2_get_link_freq() uses V4L2_CID_LINK_FREQ first, and falls back
>> + * to V4L2_CID_PIXEL_RATE if V4L2_CID_LINK_FREQ is not available.
>> + *
>> + * With multistream input there is no single pixel rate, and thus we
>> + * cannot use V4L2_CID_PIXEL_RATE, so we pass 0 as the bpp which
>> + * causes v4l2_get_link_freq() to return an error if it falls back to
>> + * V4L2_CID_PIXEL_RATE.
>> + */
>> +
>> state = v4l2_subdev_get_locked_active_state(&phy->subdev);
>>
>> - fmt = v4l2_subdev_get_pad_format(&phy->subdev, state, CAL_CAMERARX_PAD_SINK);
>> + if (state->routing.num_routes > 1) {
>> + bpp = 0;
>> + } else {
>> + struct v4l2_subdev_route *route = &state->routing.routes[0];
>> + const struct cal_format_info *fmtinfo;
>> + struct v4l2_mbus_framefmt *fmt;
>>
>> - fmtinfo = cal_format_by_code(fmt->code);
>> - if (!fmtinfo)
>> - return -EINVAL;
>> + fmt = v4l2_subdev_state_get_stream_format(
>> + state, route->sink_pad, route->sink_stream);
>>
>> - bpp = fmtinfo->bpp;
>> + fmtinfo = cal_format_by_code(fmt->code);
>> + if (!fmtinfo)
>> + return -EINVAL;
>> +
>> + bpp = fmtinfo->bpp;
>> + }
>>
>> freq = v4l2_get_link_freq(phy->source->ctrl_handler, bpp, 2 * num_lanes);
>> if (freq < 0) {
>> @@ -284,15 +301,32 @@ static void cal_camerarx_ppi_disable(struct cal_camerarx *phy)
>> 0, CAL_CSI2_PPI_CTRL_IF_EN_MASK);
>> }
>>
>> -static int cal_camerarx_start(struct cal_camerarx *phy)
>> +static int cal_camerarx_start(struct cal_camerarx *phy, u32 sink_stream)
>> {
>> + struct media_pad *remote_pad;
>> s64 link_freq;
>> u32 sscounter;
>> u32 val;
>> int ret;
>>
>> + remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
>> +
>> + /*
>> + * We need to enable the PHY hardware when enabling the first stream,
>> + * but for the following streams we just propagate the enable_streams
>> + * to the source.
>> + */
>> +
>> if (phy->enable_count > 0) {
>> + ret = v4l2_subdev_enable_streams(phy->source, remote_pad->index,
>> + BIT(sink_stream));
>> + if (ret) {
>> + phy_err(phy, "enable streams failed in source: %d\n", ret);
>> + return ret;
>> + }
>> +
>> phy->enable_count++;
>> +
>> return 0;
>> }
>>
>> @@ -394,7 +428,8 @@ static int cal_camerarx_start(struct cal_camerarx *phy)
>> * Start the source to enable the CSI-2 HS clock. We can now wait for
>> * CSI-2 PHY reset to complete.
>> */
>> - ret = v4l2_subdev_call(phy->source, video, s_stream, 1);
>> + ret = v4l2_subdev_enable_streams(phy->source, remote_pad->index,
>> + BIT(sink_stream));
>> if (ret) {
>> v4l2_subdev_call(phy->source, core, s_power, 0);
>> cal_camerarx_disable_irqs(phy);
>> @@ -425,12 +460,22 @@ static int cal_camerarx_start(struct cal_camerarx *phy)
>> return 0;
>> }
>>
>> -static void cal_camerarx_stop(struct cal_camerarx *phy)
>> +static void cal_camerarx_stop(struct cal_camerarx *phy, u32 sink_stream)
>> {
>> + struct media_pad *remote_pad;
>> int ret;
>>
>> - if (--phy->enable_count > 0)
>> + remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
>> +
>> + if (--phy->enable_count > 0) {
>> + ret = v4l2_subdev_disable_streams(phy->source,
>> + remote_pad->index,
>> + BIT(sink_stream));
>> + if (ret)
>> + phy_err(phy, "stream off failed in subdev\n");
>> +
>> return;
>> + }
>>
>> cal_camerarx_ppi_disable(phy);
>>
>> @@ -450,7 +495,9 @@ static void cal_camerarx_stop(struct cal_camerarx *phy)
>> /* Disable the phy */
>> cal_camerarx_disable(phy);
>>
>> - if (v4l2_subdev_call(phy->source, video, s_stream, 0))
>> + ret = v4l2_subdev_disable_streams(phy->source, remote_pad->index,
>> + BIT(sink_stream));
>> + if (ret)
>> phy_err(phy, "stream off failed in subdev\n");
>>
>> ret = v4l2_subdev_call(phy->source, core, s_power, 0);
>> @@ -599,30 +646,56 @@ static inline struct cal_camerarx *to_cal_camerarx(struct v4l2_subdev *sd)
>> return container_of(sd, struct cal_camerarx, subdev);
>> }
>>
>> -static int cal_camerarx_sd_s_stream(struct v4l2_subdev *sd, int enable)
>> +struct cal_camerarx *
>> +cal_camerarx_get_phy_from_entity(struct media_entity *entity)
>> +{
>> + struct v4l2_subdev *sd;
>> +
>> + sd = media_entity_to_v4l2_subdev(entity);
>> + if (!sd)
>> + return NULL;
>> +
>> + return to_cal_camerarx(sd);
>> +}
>> +
>> +static int cal_camerarx_sd_enable_streams(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_state *state,
>> + u32 pad, u64 streams_mask)
>> {
>> struct cal_camerarx *phy = to_cal_camerarx(sd);
>> - struct v4l2_subdev_state *state;
>> - int ret = 0;
>> + u32 sink_stream;
>> + int ret;
>>
>> - state = v4l2_subdev_lock_and_get_active_state(sd);
>> + ret = v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0,
>> + NULL, &sink_stream);
>> + if (ret)
>> + return ret;
>>
>> - if (enable)
>> - ret = cal_camerarx_start(phy);
>> - else
>> - cal_camerarx_stop(phy);
>> + return cal_camerarx_start(phy, sink_stream);
>> +}
>>
>> - v4l2_subdev_unlock_state(state);
>> +static int cal_camerarx_sd_disable_streams(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_state *state,
>> + u32 pad, u64 streams_mask)
>> +{
>> + struct cal_camerarx *phy = to_cal_camerarx(sd);
>> + u32 sink_stream;
>> + int ret;
>>
>> - return ret;
>> + ret = v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0,
>> + NULL, &sink_stream);
>> + if (ret)
>> + return ret;
>> +
>> + cal_camerarx_stop(phy, sink_stream);
>> +
>> + return 0;
>> }
>>
>> static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
>> struct v4l2_subdev_state *state,
>> struct v4l2_subdev_mbus_code_enum *code)
>> {
>> - struct cal_camerarx *phy = to_cal_camerarx(sd);
>> -
>> /* No transcoding, source and sink codes must match. */
>> if (cal_rx_pad_is_source(code->pad)) {
>> struct v4l2_mbus_framefmt *fmt;
>> @@ -630,8 +703,12 @@ static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
>> if (code->index > 0)
>> return -EINVAL;
>>
>> - fmt = v4l2_subdev_get_pad_format(&phy->subdev, state,
>> - CAL_CAMERARX_PAD_SINK);
>> + fmt = v4l2_subdev_state_get_opposite_stream_format(state,
>> + code->pad,
>> + code->stream);
>> + if (!fmt)
>> + return -EINVAL;
>> +
>> code->code = fmt->code;
>> } else {
>> if (code->index >= cal_num_formats)
>> @@ -656,8 +733,12 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
>> if (cal_rx_pad_is_source(fse->pad)) {
>> struct v4l2_mbus_framefmt *fmt;
>>
>> - fmt = v4l2_subdev_get_pad_format(sd, state,
>> - CAL_CAMERARX_PAD_SINK);
>> + fmt = v4l2_subdev_state_get_opposite_stream_format(state,
>> + fse->pad,
>> + fse->stream);
>> + if (!fmt)
>> + return -EINVAL;
>> +
>> if (fse->code != fmt->code)
>> return -EINVAL;
>>
>> @@ -713,36 +794,78 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
>>
>> /* Store the format and propagate it to the source pad. */
>>
>> - fmt = v4l2_subdev_get_pad_format(sd, state, CAL_CAMERARX_PAD_SINK);
>> + fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
>> + format->stream);
>> + if (!fmt)
>> + return -EINVAL;
>> +
>> *fmt = format->format;
>>
>> - fmt = v4l2_subdev_get_pad_format(sd, state,
>> - CAL_CAMERARX_PAD_FIRST_SOURCE);
>> + fmt = v4l2_subdev_state_get_opposite_stream_format(state, format->pad,
>> + format->stream);
>> + if (!fmt)
>> + return -EINVAL;
>> +
>> *fmt = format->format;
>>
>> return 0;
>> }
>>
>> +static int cal_camerarx_set_routing(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_state *state,
>> + struct v4l2_subdev_krouting *routing)
>> +{
>> + static const struct v4l2_mbus_framefmt format = {
>> + .width = 640,
>> + .height = 480,
>> + .code = MEDIA_BUS_FMT_UYVY8_1X16,
>> + .field = V4L2_FIELD_NONE,
>> + .colorspace = V4L2_COLORSPACE_SRGB,
>> + .ycbcr_enc = V4L2_YCBCR_ENC_601,
>> + .quantization = V4L2_QUANTIZATION_LIM_RANGE,
>> + .xfer_func = V4L2_XFER_FUNC_SRGB,
>> + };
>> + int ret;
>> +
>> + ret = v4l2_subdev_routing_validate(sd, routing,
>> + V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 |
>> + V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING);
>> + if (ret)
>> + return ret;
>> +
>> + ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
>> + if (ret)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +static int cal_camerarx_sd_set_routing(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_state *state,
>> + enum v4l2_subdev_format_whence which,
>> + struct v4l2_subdev_krouting *routing)
>> +{
>> + return cal_camerarx_set_routing(sd, state, routing);
>> +}
>> +
>> static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
>> struct v4l2_subdev_state *state)
>> {
>> - struct v4l2_subdev_format format = {
>> - .which = state ? V4L2_SUBDEV_FORMAT_TRY
>> - : V4L2_SUBDEV_FORMAT_ACTIVE,
>> - .pad = CAL_CAMERARX_PAD_SINK,
>> - .format = {
>> - .width = 640,
>> - .height = 480,
>> - .code = MEDIA_BUS_FMT_UYVY8_1X16,
>> - .field = V4L2_FIELD_NONE,
>> - .colorspace = V4L2_COLORSPACE_SRGB,
>> - .ycbcr_enc = V4L2_YCBCR_ENC_601,
>> - .quantization = V4L2_QUANTIZATION_LIM_RANGE,
>> - .xfer_func = V4L2_XFER_FUNC_SRGB,
>> - },
>> + struct v4l2_subdev_route routes[] = { {
>> + .sink_pad = 0,
>> + .sink_stream = 0,
>> + .source_pad = 1,
>> + .source_stream = 0,
>> + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
>> + } };
>> +
>> + struct v4l2_subdev_krouting routing = {
>> + .num_routes = 1,
>> + .routes = routes,
>> };
>>
>> - return cal_camerarx_sd_set_fmt(sd, state, &format);
>> + /* Initialize routing to single route to the fist source pad */
>> + return cal_camerarx_set_routing(sd, state, &routing);
>> }
>>
>> static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>> @@ -751,54 +874,76 @@ static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>> struct cal_camerarx *phy = to_cal_camerarx(sd);
>> struct v4l2_mbus_frame_desc remote_desc;
>> const struct media_pad *remote_pad;
>> + struct v4l2_subdev_state *state;
>> + u32 sink_stream;
>> + unsigned int i;
>> int ret;
>>
>> + state = v4l2_subdev_lock_and_get_active_state(sd);
>> +
>> + ret = v4l2_subdev_routing_find_opposite_end(&state->routing,
>> + pad, 0,
>> + NULL, &sink_stream);
>> + if (ret)
>> + goto out_unlock;
>> +
>> remote_pad = media_pad_remote_pad_first(&phy->pads[CAL_CAMERARX_PAD_SINK]);
>> - if (!remote_pad)
>> - return -EPIPE;
>> + if (!remote_pad) {
>> + ret = -EPIPE;
>> + goto out_unlock;
>> + }
>>
>> ret = v4l2_subdev_call(phy->source, pad, get_frame_desc,
>> remote_pad->index, &remote_desc);
>> if (ret)
>> - return ret;
>> + goto out_unlock;
>>
>> if (remote_desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
>> dev_err(phy->cal->dev,
>> "Frame descriptor does not describe CSI-2 link");
>
> I'm all for using standard debug functions, but the rest of the driver
> uses cal_err etc. As long as this is intentional, I'm fine with it
Ah, right. I just automatically write dev_err/dev_dbg...
>> - return -EINVAL;
>> + ret = -EINVAL;
>> + goto out_unlock;
>> + }
>> +
>> + for (i = 0; i < remote_desc.num_entries; i++) {
>> + if (remote_desc.entry[i].stream == sink_stream)
>> + break;
>> }
>>
>> - if (remote_desc.num_entries > 1)
>> - dev_dbg(phy->cal->dev,
>> - "Multiple streams not supported in remote frame descriptor, using the first one\n");
>> + if (i == remote_desc.num_entries) {
>
> I would have kept an error message here, or do you think this cannot
> happen, or is it possible that a wrong implementation on the
> transmitter's get_frame_desc could trigger it ?
Hmm. Well, I think the only way this could happen if there's a
driver/framework bug. The likely case would be that the source device
provides streams, but its get_frame_desc doesn't actually describe the
streams.
I can add a debug print.
Tomi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/4] media: ti: cal: Implement get_frame_desc for camera-rx
2023-03-01 10:05 ` Tomi Valkeinen
@ 2023-03-01 10:51 ` Jacopo Mondi
0 siblings, 0 replies; 14+ messages in thread
From: Jacopo Mondi @ 2023-03-01 10:51 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jacopo Mondi, linux-media, Laurent Pinchart, Sakari Ailus,
Kieran Bingham, Jai Luthra, Vaishnav Achath
Hi Tomi
On Wed, Mar 01, 2023 at 12:05:45PM +0200, Tomi Valkeinen wrote:
> 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@ideasonboard.com>
> > > ---
> > > 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).
>
Yeah I was thinking about some helper that explicitly fetch the DT and
VC transported on the camera-rx pad connected to this CAL instance.
But I agree the current design works, and if you're fine with that I
am as well :)
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks!
>
> Tomi
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-03-01 10:51 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-28 17:16 [PATCH v2 0/4] media: ti: cal: Streams support Tomi Valkeinen
2023-02-28 17:16 ` [PATCH v2 1/4] media: ti: cal: Clean up mbus formats uses Tomi Valkeinen
2023-03-01 9:17 ` Jacopo Mondi
2023-02-28 17:16 ` [PATCH v2 2/4] media: ti: cal: Use subdev state Tomi Valkeinen
2023-03-01 9:31 ` Jacopo Mondi
2023-03-01 9:34 ` Tomi Valkeinen
2023-03-01 9:38 ` Tomi Valkeinen
2023-02-28 17:16 ` [PATCH v2 3/4] media: ti: cal: Implement get_frame_desc for camera-rx Tomi Valkeinen
2023-03-01 9:40 ` Jacopo Mondi
2023-03-01 10:05 ` Tomi Valkeinen
2023-03-01 10:51 ` Jacopo Mondi
2023-02-28 17:16 ` [PATCH v2 4/4] media: ti: cal: Add multiplexed streams support Tomi Valkeinen
2023-03-01 9:58 ` Jacopo Mondi
2023-03-01 10:10 ` Tomi Valkeinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).