From 7ca3f841e2a66d2145bedc06dbcce5044725f622 Mon Sep 17 00:00:00 2001 From: Ondrej Jirman Date: Sat, 13 Jan 2024 21:24:24 +0100 Subject: media: sun6i-csi: subdev: Use subdev active state to store active format The sun6i_csi_bridge structure stores the active format as a struct v4l2_mbus_framefmt instance. Now that the V4L2 subdev core supports managing the active state, use it to replace the manually-managed format. This simplifies the implementation of the subdev operations by removing the need for a custom .get_fmt() handler. The subdev state now needs to be passed manually from the .s_stream() operation to internal functions that need to access the format. As a result, the sun6i_csi_bridge_format() helper function is now unused and can be dropped. The .init_state() and .set_fmt() operations also need to be updated accordingly. As a result, .set_fmt() now correctly propagates the format from the sink pad to the source pad. Signed-off-by: Laurent Pinchart (cherry picked from commit b14038f024ad84473ed79f3baecc2da9ca16d9a5) --- .../sunxi/sun6i-csi/sun6i_csi_bridge.c | 149 +++++++++--------- .../sunxi/sun6i-csi/sun6i_csi_bridge.h | 2 - 2 files changed, 73 insertions(+), 78 deletions(-) diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c index 2303e5d2baee..92290876a8b3 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c @@ -13,17 +13,6 @@ #include "sun6i_csi_bridge.h" #include "sun6i_csi_reg.h" -/* Helpers */ - -static void sun6i_csi_bridge_format(struct sun6i_csi_device *csi_dev, - u32 *mbus_code, u32 *field) -{ - if (mbus_code) - *mbus_code = csi_dev->bridge.mbus_format.code; - if (field) - *field = csi_dev->bridge.mbus_format.field; -} - /* Format */ static const struct sun6i_csi_bridge_format sun6i_csi_bridge_formats[] = { @@ -217,7 +206,8 @@ static void sun6i_csi_bridge_disable(struct sun6i_csi_device *csi_dev) } static void -sun6i_csi_bridge_configure_parallel(struct sun6i_csi_device *csi_dev) +sun6i_csi_bridge_configure_parallel(struct sun6i_csi_device *csi_dev, + struct v4l2_subdev_state *state) { struct device *dev = csi_dev->dev; struct regmap *regmap = csi_dev->regmap; @@ -225,14 +215,14 @@ sun6i_csi_bridge_configure_parallel(struct sun6i_csi_device *csi_dev) &csi_dev->bridge.source_parallel.endpoint; unsigned char bus_width = endpoint->bus.parallel.bus_width; unsigned int flags = endpoint->bus.parallel.flags; - u32 field; + const struct v4l2_mbus_framefmt *sink_format; u32 value = SUN6I_CSI_IF_CFG_IF_CSI; - sun6i_csi_bridge_format(csi_dev, NULL, &field); + sink_format = v4l2_subdev_state_get_format(state, SUN6I_CSI_BRIDGE_PAD_SINK); - if (field == V4L2_FIELD_INTERLACED || - field == V4L2_FIELD_INTERLACED_TB || - field == V4L2_FIELD_INTERLACED_BT) + if (sink_format->field == V4L2_FIELD_INTERLACED || + sink_format->field == V4L2_FIELD_INTERLACED_TB || + sink_format->field == V4L2_FIELD_INTERLACED_BT) value |= SUN6I_CSI_IF_CFG_SRC_TYPE_INTERLACED | SUN6I_CSI_IF_CFG_FIELD_DT_PCLK_SHIFT(1) | SUN6I_CSI_IF_CFG_FIELD_DT_FIELD_VSYNC; @@ -308,17 +298,18 @@ sun6i_csi_bridge_configure_parallel(struct sun6i_csi_device *csi_dev) } static void -sun6i_csi_bridge_configure_mipi_csi2(struct sun6i_csi_device *csi_dev) +sun6i_csi_bridge_configure_mipi_csi2(struct sun6i_csi_device *csi_dev, + struct v4l2_subdev_state *state) { + const struct v4l2_mbus_framefmt *sink_format; struct regmap *regmap = csi_dev->regmap; u32 value = SUN6I_CSI_IF_CFG_IF_MIPI; - u32 field; - sun6i_csi_bridge_format(csi_dev, NULL, &field); + sink_format = v4l2_subdev_state_get_format(state, SUN6I_CSI_BRIDGE_PAD_SINK); - if (field == V4L2_FIELD_INTERLACED || - field == V4L2_FIELD_INTERLACED_TB || - field == V4L2_FIELD_INTERLACED_BT) + if (sink_format->field == V4L2_FIELD_INTERLACED || + sink_format->field == V4L2_FIELD_INTERLACED_TB || + sink_format->field == V4L2_FIELD_INTERLACED_BT) value |= SUN6I_CSI_IF_CFG_SRC_TYPE_INTERLACED; else value |= SUN6I_CSI_IF_CFG_SRC_TYPE_PROGRESSIVE; @@ -326,19 +317,20 @@ sun6i_csi_bridge_configure_mipi_csi2(struct sun6i_csi_device *csi_dev) regmap_write(regmap, SUN6I_CSI_IF_CFG_REG, value); } -static void sun6i_csi_bridge_configure_format(struct sun6i_csi_device *csi_dev) +static void sun6i_csi_bridge_configure_format(struct sun6i_csi_device *csi_dev, + struct v4l2_subdev_state *state) { struct regmap *regmap = csi_dev->regmap; bool capture_streaming = csi_dev->capture.state.streaming; const struct sun6i_csi_bridge_format *bridge_format; const struct sun6i_csi_capture_format *capture_format; - u32 mbus_code, field, pixelformat; + const struct v4l2_mbus_framefmt *sink_format; + u32 pixelformat; u8 input_format, input_yuv_seq, output_format; u32 value = 0; - sun6i_csi_bridge_format(csi_dev, &mbus_code, &field); - - bridge_format = sun6i_csi_bridge_format_find(mbus_code); + sink_format = v4l2_subdev_state_get_format(state, SUN6I_CSI_BRIDGE_PAD_SINK); + bridge_format = sun6i_csi_bridge_format_find(sink_format->code); if (WARN_ON(!bridge_format)) return; @@ -358,9 +350,9 @@ static void sun6i_csi_bridge_configure_format(struct sun6i_csi_device *csi_dev) if (capture_format->input_yuv_seq_invert) input_yuv_seq = bridge_format->input_yuv_seq_invert; - if (field == V4L2_FIELD_INTERLACED || - field == V4L2_FIELD_INTERLACED_TB || - field == V4L2_FIELD_INTERLACED_BT) + if (sink_format->field == V4L2_FIELD_INTERLACED || + sink_format->field == V4L2_FIELD_INTERLACED_TB || + sink_format->field == V4L2_FIELD_INTERLACED_BT) output_format = capture_format->output_format_field; else output_format = capture_format->output_format_frame; @@ -371,9 +363,9 @@ static void sun6i_csi_bridge_configure_format(struct sun6i_csi_device *csi_dev) value |= SUN6I_CSI_CH_CFG_INPUT_FMT(input_format); value |= SUN6I_CSI_CH_CFG_INPUT_YUV_SEQ(input_yuv_seq); - if (field == V4L2_FIELD_TOP) + if (sink_format->field == V4L2_FIELD_TOP) value |= SUN6I_CSI_CH_CFG_FIELD_SEL_FIELD0; - else if (field == V4L2_FIELD_BOTTOM) + else if (sink_format->field == V4L2_FIELD_BOTTOM) value |= SUN6I_CSI_CH_CFG_FIELD_SEL_FIELD1; else value |= SUN6I_CSI_CH_CFG_FIELD_SEL_EITHER; @@ -382,16 +374,17 @@ static void sun6i_csi_bridge_configure_format(struct sun6i_csi_device *csi_dev) } static void sun6i_csi_bridge_configure(struct sun6i_csi_device *csi_dev, - struct sun6i_csi_bridge_source *source) + struct sun6i_csi_bridge_source *source, + struct v4l2_subdev_state *state) { struct sun6i_csi_bridge *bridge = &csi_dev->bridge; if (source == &bridge->source_parallel) - sun6i_csi_bridge_configure_parallel(csi_dev); + sun6i_csi_bridge_configure_parallel(csi_dev, state); else - sun6i_csi_bridge_configure_mipi_csi2(csi_dev); + sun6i_csi_bridge_configure_mipi_csi2(csi_dev, state); - sun6i_csi_bridge_configure_format(csi_dev); + sun6i_csi_bridge_configure_format(csi_dev, state); } /* V4L2 Subdev */ @@ -406,6 +399,7 @@ static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on) struct sun6i_csi_bridge_source *source; struct v4l2_subdev *source_subdev; struct media_pad *remote_pad; + struct v4l2_subdev_state *state; int ret; /* Source */ @@ -441,8 +435,9 @@ static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on) sun6i_csi_bridge_irq_clear(csi_dev); /* Configure */ - - sun6i_csi_bridge_configure(csi_dev, source); + state = v4l2_subdev_lock_and_get_active_state(subdev); + sun6i_csi_bridge_configure(csi_dev, source, state); + v4l2_subdev_unlock_state(state); if (capture_streaming) sun6i_csi_capture_configure(csi_dev); @@ -495,13 +490,9 @@ sun6i_csi_bridge_mbus_format_prepare(struct v4l2_mbus_framefmt *mbus_format) static int sun6i_csi_bridge_init_state(struct v4l2_subdev *subdev, struct v4l2_subdev_state *state) { - struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev); unsigned int pad = SUN6I_CSI_BRIDGE_PAD_SINK; struct v4l2_mbus_framefmt *mbus_format = v4l2_subdev_state_get_format(state, pad); - struct mutex *lock = &csi_dev->bridge.lock; - - mutex_lock(lock); mbus_format->code = sun6i_csi_bridge_formats[0].mbus_code; mbus_format->width = 1280; @@ -509,8 +500,6 @@ static int sun6i_csi_bridge_init_state(struct v4l2_subdev *subdev, sun6i_csi_bridge_mbus_format_prepare(mbus_format); - mutex_unlock(lock); - return 0; } @@ -527,53 +516,60 @@ sun6i_csi_bridge_enum_mbus_code(struct v4l2_subdev *subdev, return 0; } -static int sun6i_csi_bridge_get_fmt(struct v4l2_subdev *subdev, - struct v4l2_subdev_state *state, - struct v4l2_subdev_format *format) -{ - struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev); - struct v4l2_mbus_framefmt *mbus_format = &format->format; - struct mutex *lock = &csi_dev->bridge.lock; - - mutex_lock(lock); - - if (format->which == V4L2_SUBDEV_FORMAT_TRY) - *mbus_format = *v4l2_subdev_state_get_format(state, - format->pad); - else - *mbus_format = csi_dev->bridge.mbus_format; - - mutex_unlock(lock); - - return 0; -} - static int sun6i_csi_bridge_set_fmt(struct v4l2_subdev *subdev, struct v4l2_subdev_state *state, struct v4l2_subdev_format *format) { - struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev); - struct v4l2_mbus_framefmt *mbus_format = &format->format; - struct mutex *lock = &csi_dev->bridge.lock; + struct v4l2_mbus_framefmt *sink_format; + struct v4l2_mbus_framefmt *src_format; - mutex_lock(lock); + /* + * As the source format can't change independently of the sink format, + * a set operation on the source is a get. + */ + if (format->pad == SUN6I_CSI_BRIDGE_PAD_SOURCE) + return v4l2_subdev_get_fmt(subdev, state, format); - sun6i_csi_bridge_mbus_format_prepare(mbus_format); + sink_format = v4l2_subdev_state_get_format(state, SUN6I_CSI_BRIDGE_PAD_SINK); + /* if (format->which == V4L2_SUBDEV_FORMAT_TRY) *v4l2_subdev_state_get_format(state, format->pad) = *mbus_format; + */ + + if (!sun6i_csi_bridge_format_find(format->format.code)) + sink_format->code = sun6i_csi_bridge_formats[0].mbus_code; else - csi_dev->bridge.mbus_format = *mbus_format; + sink_format->code = format->format.code; + + sink_format->width = format->format.width; + sink_format->height = format->format.height; + + /* Accept any colorspace as the CSI bridge isn't colorspace-aware. */ + sink_format->colorspace = format->format.colorspace; + sink_format->xfer_func = format->format.xfer_func; + sink_format->ycbcr_enc = format->format.ycbcr_enc; + sink_format->quantization = format->format.quantization; + + sink_format->field = V4L2_FIELD_NONE; - mutex_unlock(lock); + format->format = *sink_format; + + /* + * Propagate the format to the source pad. The CSI bridge can't + * transcode, the format on the source pad always matches the sink pad + * exactly. + */ + src_format = v4l2_subdev_state_get_format(state, SUN6I_CSI_BRIDGE_PAD_SOURCE); + *src_format = *sink_format; return 0; } static const struct v4l2_subdev_pad_ops sun6i_csi_bridge_pad_ops = { .enum_mbus_code = sun6i_csi_bridge_enum_mbus_code, - .get_fmt = sun6i_csi_bridge_get_fmt, + .get_fmt = v4l2_subdev_get_fmt, .set_fmt = sun6i_csi_bridge_set_fmt, }; @@ -771,8 +767,6 @@ int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev) }; int ret; - mutex_init(&bridge->lock); - /* V4L2 Subdev */ v4l2_subdev_init(subdev, &sun6i_csi_bridge_subdev_ops); @@ -801,6 +795,9 @@ int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev) return ret; /* V4L2 Subdev */ + ret = v4l2_subdev_init_finalize(subdev); + if (ret) + goto error_media_entity; if (csi_dev->isp_available) ret = v4l2_async_register_subdev(subdev); diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h index 722b633b7893..a5b0a6f064dd 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h @@ -42,8 +42,6 @@ struct sun6i_csi_bridge { struct v4l2_subdev subdev; struct v4l2_async_notifier notifier; struct media_pad pads[2]; - struct v4l2_mbus_framefmt mbus_format; - struct mutex lock; /* Mbus format lock. */ struct sun6i_csi_bridge_source source_parallel; struct sun6i_csi_bridge_source source_mipi_csi2; -- 2.35.3