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_cfg() 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 <laurent.pinchart@xxxxxxxxxxxxxxxx> --- .../sunxi/sun6i-csi/sun6i_csi_bridge.c | 202 ++++++++---------- .../sunxi/sun6i-csi/sun6i_csi_bridge.h | 2 - 2 files changed, 95 insertions(+), 109 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 a149a0d4e601..d3a8b18ff348 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,15 @@ 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_get_pad_format(&csi_dev->bridge.subdev, 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 +299,19 @@ 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_get_pad_format(&csi_dev->bridge.subdev, 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 +319,21 @@ 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_get_pad_format(&csi_dev->bridge.subdev, state, + SUN6I_CSI_BRIDGE_PAD_SINK); + bridge_format = sun6i_csi_bridge_format_find(sink_format->code); if (WARN_ON(!bridge_format)) return; @@ -358,9 +353,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 +366,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 +377,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 +402,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 +438,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); @@ -480,40 +478,6 @@ static const struct v4l2_subdev_video_ops sun6i_csi_bridge_video_ops = { .s_stream = sun6i_csi_bridge_s_stream, }; -static void -sun6i_csi_bridge_mbus_format_prepare(struct v4l2_mbus_framefmt *mbus_format) -{ - if (!sun6i_csi_bridge_format_find(mbus_format->code)) - mbus_format->code = sun6i_csi_bridge_formats[0].mbus_code; - - mbus_format->field = V4L2_FIELD_NONE; - mbus_format->colorspace = V4L2_COLORSPACE_RAW; - mbus_format->quantization = V4L2_QUANTIZATION_DEFAULT; - mbus_format->xfer_func = V4L2_XFER_FUNC_DEFAULT; -} - -static int sun6i_csi_bridge_init_cfg(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_get_try_format(subdev, 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; - mbus_format->height = 720; - - sun6i_csi_bridge_mbus_format_prepare(mbus_format); - - mutex_unlock(lock); - - return 0; -} - static int sun6i_csi_bridge_enum_mbus_code(struct v4l2_subdev *subdev, struct v4l2_subdev_state *state, @@ -527,54 +491,77 @@ 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_get_try_format(subdev, 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_get_pad_format(subdev, state, + SUN6I_CSI_BRIDGE_PAD_SINK); - if (format->which == V4L2_SUBDEV_FORMAT_TRY) - *v4l2_subdev_get_try_format(subdev, 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; - mutex_unlock(lock); + 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; + + 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_get_pad_format(subdev, state, + SUN6I_CSI_BRIDGE_PAD_SOURCE); + *src_format = *sink_format; return 0; } +static int sun6i_csi_bridge_init_cfg(struct v4l2_subdev *subdev, + struct v4l2_subdev_state *state) +{ + struct v4l2_subdev_format format = { + .pad = SUN6I_CSI_BRIDGE_PAD_SINK, + .format = { + .code = sun6i_csi_bridge_formats[0].mbus_code, + .width = 1280, + .height = 720, + .field = V4L2_FIELD_NONE, + .colorspace = V4L2_COLORSPACE_RAW, + .ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT, + .quantization = V4L2_QUANTIZATION_DEFAULT, + .xfer_func = V4L2_XFER_FUNC_DEFAULT, + }, + }; + + return sun6i_csi_bridge_set_fmt(subdev, state, &format); +} + static const struct v4l2_subdev_pad_ops sun6i_csi_bridge_pad_ops = { .init_cfg = sun6i_csi_bridge_init_cfg, .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, }; @@ -768,8 +755,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); @@ -797,6 +782,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 b827ab1be011..7754eaaab743 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; -- Regards, Laurent Pinchart