| Message ID | 20220205185429.2278860-31-paul.kocialkowski@bootlin.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Allwinner A31/A83T MIPI CSI-2 Support and A31 ISP Support | expand |
On Sat, Feb 05, 2022 at 07:53:53PM +0100, Paul Kocialkowski wrote: > Introduce a bridge v4l2 subdev to prepare for separation between the > processing part (bridge) and the dma engine, which is required to > properly support ths isp workflow later on. > > Currently the bridge just manages fwnode mapping to media pads, > using an async notifier (which was previously in the main code). > The s_stream video op just forwards to the connected v4l2 subdev > (sensor or MIPI CSI-2 bridge). > > The video capture device is now registered after the bridge and > attaches to it with a media link. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> There's a bunch of checkpatch --strict warnings that need to be fixed Maxime
Hi, On Wed 09 Feb 22, 10:24, Maxime Ripard wrote: > On Sat, Feb 05, 2022 at 07:53:53PM +0100, Paul Kocialkowski wrote: > > Introduce a bridge v4l2 subdev to prepare for separation between the > > processing part (bridge) and the dma engine, which is required to > > properly support ths isp workflow later on. > > > > Currently the bridge just manages fwnode mapping to media pads, > > using an async notifier (which was previously in the main code). > > The s_stream video op just forwards to the connected v4l2 subdev > > (sensor or MIPI CSI-2 bridge). > > > > The video capture device is now registered after the bridge and > > attaches to it with a media link. > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > There's a bunch of checkpatch --strict warnings that need to be fixed Yes so it turns out these are adaptations to the existing video code which has these warnings already merged. They are cleaned up later on in a dedicated commit, but since it's not the topic of this change (which is a logic change) I kept the code as it is. What do you think? Cheers, Paul
On Fri, Feb 11, 2022 at 04:43:51PM +0100, Paul Kocialkowski wrote: > Hi, > > On Wed 09 Feb 22, 10:24, Maxime Ripard wrote: > > On Sat, Feb 05, 2022 at 07:53:53PM +0100, Paul Kocialkowski wrote: > > > Introduce a bridge v4l2 subdev to prepare for separation between the > > > processing part (bridge) and the dma engine, which is required to > > > properly support ths isp workflow later on. > > > > > > Currently the bridge just manages fwnode mapping to media pads, > > > using an async notifier (which was previously in the main code). > > > The s_stream video op just forwards to the connected v4l2 subdev > > > (sensor or MIPI CSI-2 bridge). > > > > > > The video capture device is now registered after the bridge and > > > attaches to it with a media link. > > > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > > > There's a bunch of checkpatch --strict warnings that need to be fixed > > Yes so it turns out these are adaptations to the existing video code > which has these warnings already merged. They are cleaned up later on > in a dedicated commit, but since it's not the topic of this change > (which is a logic change) I kept the code as it is. > > What do you think? + async_subdev = v4l2_async_nf_add_fwnode_remote(notifier, handle, + struct v4l2_async_subdev); CHECK: Alignment should match open parenthesis This one at least is introduced by your patch Maxime
Hi Paul, On Sat, Feb 05, 2022 at 07:53:53PM +0100, Paul Kocialkowski wrote: > Introduce a bridge v4l2 subdev to prepare for separation between the > processing part (bridge) and the dma engine, which is required to > properly support ths isp workflow later on. > > Currently the bridge just manages fwnode mapping to media pads, > using an async notifier (which was previously in the main code). > The s_stream video op just forwards to the connected v4l2 subdev > (sensor or MIPI CSI-2 bridge). > > The video capture device is now registered after the bridge and > attaches to it with a media link. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > .../media/platform/sunxi/sun6i-csi/Makefile | 2 +- > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 156 +----- > .../platform/sunxi/sun6i-csi/sun6i_csi.h | 12 +- > .../sunxi/sun6i-csi/sun6i_csi_bridge.c | 473 ++++++++++++++++++ > .../sunxi/sun6i-csi/sun6i_csi_bridge.h | 44 ++ > .../platform/sunxi/sun6i-csi/sun6i_video.c | 19 + > 6 files changed, 571 insertions(+), 135 deletions(-) > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/Makefile b/drivers/media/platform/sunxi/sun6i-csi/Makefile > index e7e315347804..7a699580a641 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/Makefile > +++ b/drivers/media/platform/sunxi/sun6i-csi/Makefile > @@ -1,4 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0-only > -sun6i-csi-y += sun6i_video.o sun6i_csi.o > +sun6i-csi-y += sun6i_video.o sun6i_csi.o sun6i_csi_bridge.o > > obj-$(CONFIG_VIDEO_SUN6I_CSI) += sun6i-csi.o > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > index c8fe31cc38b5..a1847ae3e88e 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c > @@ -34,16 +34,17 @@ > bool sun6i_csi_is_format_supported(struct sun6i_csi_device *csi_dev, > u32 pixformat, u32 mbus_code) > { > - struct sun6i_csi_v4l2 *v4l2 = &csi_dev->v4l2; > + struct v4l2_fwnode_endpoint *endpoint = > + &csi_dev->bridge.source->endpoint; > > /* > * Some video receivers have the ability to be compatible with > * 8bit and 16bit bus width. > * Identify the media bus format from device tree. > */ > - if ((v4l2->v4l2_ep.bus_type == V4L2_MBUS_PARALLEL > - || v4l2->v4l2_ep.bus_type == V4L2_MBUS_BT656) > - && v4l2->v4l2_ep.bus.parallel.bus_width == 16) { > + if ((endpoint->bus_type == V4L2_MBUS_PARALLEL > + || endpoint->bus_type == V4L2_MBUS_BT656) > + && endpoint->bus.parallel.bus_width == 16) { > switch (pixformat) { > case V4L2_PIX_FMT_NV12_16L16: > case V4L2_PIX_FMT_NV12: > @@ -328,7 +329,8 @@ static enum csi_input_seq get_csi_input_seq(struct sun6i_csi_device *csi_dev, > > static void sun6i_csi_setup_bus(struct sun6i_csi_device *csi_dev) > { > - struct v4l2_fwnode_endpoint *endpoint = &csi_dev->v4l2.v4l2_ep; > + struct v4l2_fwnode_endpoint *endpoint = > + &csi_dev->bridge.source->endpoint; > struct sun6i_csi_config *config = &csi_dev->config; > unsigned char bus_width; > u32 flags; > @@ -583,95 +585,11 @@ static const struct media_device_ops sun6i_csi_media_ops = { > > /* V4L2 */ > > -static int sun6i_csi_link_entity(struct sun6i_csi_device *csi_dev, > - struct media_entity *entity, > - struct fwnode_handle *fwnode) > +int sun6i_csi_v4l2_complete(struct sun6i_csi_device *csi_dev) > { > - struct media_entity *sink; > - struct media_pad *sink_pad; > - int src_pad_index; > - int ret; > - > - ret = media_entity_get_fwnode_pad(entity, fwnode, MEDIA_PAD_FL_SOURCE); > - if (ret < 0) { > - dev_err(csi_dev->dev, > - "%s: no source pad in external entity %s\n", __func__, > - entity->name); > - return -EINVAL; > - } > - > - src_pad_index = ret; > - > - sink = &csi_dev->video.video_dev.entity; > - sink_pad = &csi_dev->video.pad; > - > - dev_dbg(csi_dev->dev, "creating %s:%u -> %s:%u link\n", > - entity->name, src_pad_index, sink->name, sink_pad->index); > - ret = media_create_pad_link(entity, src_pad_index, sink, > - sink_pad->index, > - MEDIA_LNK_FL_ENABLED | > - MEDIA_LNK_FL_IMMUTABLE); > - if (ret < 0) { > - dev_err(csi_dev->dev, "failed to create %s:%u -> %s:%u link\n", > - entity->name, src_pad_index, > - sink->name, sink_pad->index); > - return ret; > - } > - > - return 0; > -} > - > -static int sun6i_subdev_notify_complete(struct v4l2_async_notifier *notifier) > -{ > - struct sun6i_csi_device *csi_dev = > - container_of(notifier, struct sun6i_csi_device, > - v4l2.notifier); > - struct sun6i_csi_v4l2 *v4l2 = &csi_dev->v4l2; > - struct v4l2_device *v4l2_dev = &v4l2->v4l2_dev; > - struct v4l2_subdev *sd; > - int ret; > - > - dev_dbg(csi_dev->dev, "notify complete, all subdevs registered\n"); > - > - sd = list_first_entry(&v4l2_dev->subdevs, struct v4l2_subdev, list); > - if (!sd) > - return -EINVAL; > - > - ret = sun6i_csi_link_entity(csi_dev, &sd->entity, sd->fwnode); > - if (ret < 0) > - return ret; > - > - ret = v4l2_device_register_subdev_nodes(v4l2_dev); > - if (ret < 0) > - return ret; > - > - return 0; > -} > - > -static const struct v4l2_async_notifier_operations sun6i_csi_async_ops = { > - .complete = sun6i_subdev_notify_complete, > -}; > - > -static int sun6i_csi_fwnode_parse(struct device *dev, > - struct v4l2_fwnode_endpoint *vep, > - struct v4l2_async_subdev *asd) > -{ > - struct sun6i_csi_device *csi_dev = dev_get_drvdata(dev); > + struct v4l2_device *v4l2_dev = &csi_dev->v4l2.v4l2_dev; > > - if (vep->base.port || vep->base.id) { > - dev_warn(dev, "Only support a single port with one endpoint\n"); > - return -ENOTCONN; > - } > - > - switch (vep->bus_type) { > - case V4L2_MBUS_PARALLEL: > - case V4L2_MBUS_BT656: > - csi_dev->v4l2.v4l2_ep = *vep; > - return 0; > - default: > - dev_err(dev, "Unsupported media bus type\n"); > - return -ENOTCONN; > - } > + return v4l2_device_register_subdev_nodes(v4l2_dev); > } > > static int sun6i_csi_v4l2_setup(struct sun6i_csi_device *csi_dev) > @@ -679,7 +597,6 @@ static int sun6i_csi_v4l2_setup(struct sun6i_csi_device *csi_dev) > struct sun6i_csi_v4l2 *v4l2 = &csi_dev->v4l2; > struct media_device *media_dev = &v4l2->media_dev; > struct v4l2_device *v4l2_dev = &v4l2->v4l2_dev; > - struct v4l2_async_notifier *notifier = &v4l2->notifier; > struct device *dev = csi_dev->dev; > int ret; > > @@ -720,42 +637,8 @@ static int sun6i_csi_v4l2_setup(struct sun6i_csi_device *csi_dev) > goto error_v4l2_ctrl; > } > > - /* Video */ > - > - ret = sun6i_video_setup(csi_dev); > - if (ret) > - goto error_v4l2_device; > - > - /* V4L2 Async */ > - > - v4l2_async_nf_init(notifier); > - notifier->ops = &sun6i_csi_async_ops; > - > - ret = v4l2_async_nf_parse_fwnode_endpoints(dev, notifier, > - sizeof(struct > - v4l2_async_subdev), > - sun6i_csi_fwnode_parse); > - if (ret) > - goto error_video; > - > - ret = v4l2_async_nf_register(v4l2_dev, notifier); > - if (ret) { > - dev_err(dev, "failed to register v4l2 async notifier: %d\n", > - ret); > - goto error_v4l2_async_notifier; > - } > - > return 0; > > -error_v4l2_async_notifier: > - v4l2_async_nf_cleanup(notifier); > - > -error_video: > - sun6i_video_cleanup(csi_dev); > - > -error_v4l2_device: > - v4l2_device_unregister(&v4l2->v4l2_dev); > - > error_v4l2_ctrl: > v4l2_ctrl_handler_free(&v4l2->ctrl_handler); > > @@ -771,9 +654,6 @@ static void sun6i_csi_v4l2_cleanup(struct sun6i_csi_device *csi_dev) > struct sun6i_csi_v4l2 *v4l2 = &csi_dev->v4l2; > > media_device_unregister(&v4l2->media_dev); > - v4l2_async_nf_unregister(&v4l2->notifier); > - v4l2_async_nf_cleanup(&v4l2->notifier); > - sun6i_video_cleanup(csi_dev); > v4l2_device_unregister(&v4l2->v4l2_dev); > v4l2_ctrl_handler_free(&v4l2->ctrl_handler); > media_device_cleanup(&v4l2->media_dev); > @@ -995,8 +875,22 @@ static int sun6i_csi_probe(struct platform_device *platform_dev) > if (ret) > goto error_resources; > > + ret = sun6i_csi_bridge_setup(csi_dev); > + if (ret) > + goto error_v4l2; > + > + ret = sun6i_video_setup(csi_dev); > + if (ret) > + goto error_bridge; > + > return 0; > > +error_bridge: > + sun6i_csi_bridge_cleanup(csi_dev); > + > +error_v4l2: > + sun6i_csi_v4l2_cleanup(csi_dev); > + > error_resources: > sun6i_csi_resources_cleanup(csi_dev); > > @@ -1007,6 +901,8 @@ static int sun6i_csi_remove(struct platform_device *pdev) > { > struct sun6i_csi_device *csi_dev = platform_get_drvdata(pdev); > > + sun6i_video_cleanup(csi_dev); > + sun6i_csi_bridge_cleanup(csi_dev); > sun6i_csi_v4l2_cleanup(csi_dev); > sun6i_csi_resources_cleanup(csi_dev); > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h > index 4dd83e57bafa..576c7f10289e 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h > @@ -13,11 +13,16 @@ > #include <media/v4l2-fwnode.h> > #include <media/videobuf2-v4l2.h> > > +#include "sun6i_csi_bridge.h" > #include "sun6i_video.h" > > #define SUN6I_CSI_NAME "sun6i-csi" > #define SUN6I_CSI_DESCRIPTION "Allwinner A31 CSI Device" > > +enum sun6i_csi_port { > + SUN6I_CSI_PORT_PARALLEL = 0, > +}; > + > struct sun6i_csi_buffer { > struct vb2_v4l2_buffer v4l2_buffer; > struct list_head list; > @@ -46,10 +51,6 @@ struct sun6i_csi_v4l2 { > struct v4l2_device v4l2_dev; > struct v4l2_ctrl_handler ctrl_handler; > struct media_device media_dev; > - > - struct v4l2_async_notifier notifier; > - /* video port settings */ > - struct v4l2_fwnode_endpoint v4l2_ep; > }; > > struct sun6i_csi_device { > @@ -57,6 +58,7 @@ struct sun6i_csi_device { > > struct sun6i_csi_config config; > struct sun6i_csi_v4l2 v4l2; > + struct sun6i_csi_bridge bridge; > struct sun6i_video video; > > struct regmap *regmap; > @@ -156,4 +158,6 @@ static inline int sun6i_csi_get_bpp(unsigned int pixformat) > return 0; > } > > +int sun6i_csi_v4l2_complete(struct sun6i_csi_device *csi_dev); > + > #endif /* __SUN6I_CSI_H__ */ > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c > new file mode 100644 > index 000000000000..74706d883359 > --- /dev/null > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c > @@ -0,0 +1,473 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2021-2022 Bootlin > + * Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > + */ > + > +#include <linux/pm_runtime.h> > +#include <linux/regmap.h> > +#include <media/v4l2-device.h> > +#include <media/v4l2-fwnode.h> > + > +#include "sun6i_csi.h" > +#include "sun6i_csi_bridge.h" > + > +/* Format */ > + > +static const u32 sun6i_csi_bridge_mbus_codes[] = { > + /* Bayer */ > + MEDIA_BUS_FMT_SBGGR8_1X8, > + MEDIA_BUS_FMT_SGBRG8_1X8, > + MEDIA_BUS_FMT_SGRBG8_1X8, > + MEDIA_BUS_FMT_SRGGB8_1X8, > + MEDIA_BUS_FMT_SBGGR10_1X10, > + MEDIA_BUS_FMT_SGBRG10_1X10, > + MEDIA_BUS_FMT_SGRBG10_1X10, > + MEDIA_BUS_FMT_SRGGB10_1X10, > + MEDIA_BUS_FMT_SBGGR12_1X12, > + MEDIA_BUS_FMT_SGBRG12_1X12, > + MEDIA_BUS_FMT_SGRBG12_1X12, > + MEDIA_BUS_FMT_SRGGB12_1X12, > + /* RGB */ > + MEDIA_BUS_FMT_RGB565_2X8_LE, > + MEDIA_BUS_FMT_RGB565_2X8_BE, > + /* YUV422 */ > + MEDIA_BUS_FMT_YUYV8_2X8, > + MEDIA_BUS_FMT_UYVY8_2X8, > + MEDIA_BUS_FMT_YVYU8_2X8, > + MEDIA_BUS_FMT_UYVY8_2X8, > + MEDIA_BUS_FMT_VYUY8_2X8, > + MEDIA_BUS_FMT_YUYV8_1X16, > + MEDIA_BUS_FMT_UYVY8_1X16, > + MEDIA_BUS_FMT_YVYU8_1X16, > + MEDIA_BUS_FMT_UYVY8_1X16, > + MEDIA_BUS_FMT_VYUY8_1X16, > + /* Compressed */ > + MEDIA_BUS_FMT_JPEG_1X8, > +}; > + > +static bool sun6i_csi_bridge_mbus_code_check(u32 mbus_code) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(sun6i_csi_bridge_mbus_codes); i++) > + if (sun6i_csi_bridge_mbus_codes[i] == mbus_code) > + return true; > + > + return false; > +} > + > +/* V4L2 Subdev */ > + > +static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on) > +{ > + struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev); > + struct v4l2_subdev *source_subdev; > + /* Initialize to 0 to use both in disable label (ret != 0) and off. */ > + int ret = 0; > + > + /* Source */ > + > + if (!csi_dev->bridge.source) > + return -ENODEV; > + > + source_subdev = csi_dev->bridge.source->subdev; > + > + if (!on) { > + v4l2_subdev_call(source_subdev, video, s_stream, 0); > + goto disable; > + } > + > + ret = v4l2_subdev_call(source_subdev, video, s_stream, 1); > + if (ret && ret != -ENOIOCTLCMD) > + goto disable; > + > + return 0; > + > +disable: > + csi_dev->bridge.source = NULL; > + > + return ret; > +} > + > +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_mbus_code_check(mbus_format->code)) > + mbus_format->code = sun6i_csi_bridge_mbus_codes[0]; > + > + 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) > +{ > + unsigned int pad = SUN6I_CSI_BRIDGE_PAD_SINK; > + struct v4l2_mbus_framefmt *mbus_format = > + v4l2_subdev_get_try_format(subdev, state, pad); > + > + mbus_format->code = sun6i_csi_bridge_mbus_codes[0]; > + mbus_format->width = 1280; > + mbus_format->height = 720; > + > + sun6i_csi_bridge_mbus_format_prepare(mbus_format); > + > + return 0; > +} > + > +static int > +sun6i_csi_bridge_enum_mbus_code(struct v4l2_subdev *subdev, > + struct v4l2_subdev_state *state, > + struct v4l2_subdev_mbus_code_enum *code_enum) > +{ > + if (code_enum->index >= ARRAY_SIZE(sun6i_csi_bridge_mbus_codes)) > + return -EINVAL; > + > + code_enum->code = sun6i_csi_bridge_mbus_codes[code_enum->index]; > + > + 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; > + > + 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; > + > + 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; > + > + sun6i_csi_bridge_mbus_format_prepare(mbus_format); > + > + if (format->which == V4L2_SUBDEV_FORMAT_TRY) > + *v4l2_subdev_get_try_format(subdev, state, format->pad) = > + *mbus_format; > + else > + csi_dev->bridge.mbus_format = *mbus_format; Note that the driver is responsible for serialising access to its data, i.e. you have to acquire the mutex here. > + > + return 0; > +} > + > +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, > + .set_fmt = sun6i_csi_bridge_set_fmt, > +}; > + > +const struct v4l2_subdev_ops sun6i_csi_bridge_subdev_ops = { > + .video = &sun6i_csi_bridge_video_ops, > + .pad = &sun6i_csi_bridge_pad_ops, > +}; > + > +/* Media Entity */ > + > +static int sun6i_csi_bridge_link_validate(struct media_link *link) > +{ > + struct v4l2_subdev *subdev = > + media_entity_to_v4l2_subdev(link->sink->entity); > + struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev); > + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; > + struct device *dev = csi_dev->dev; > + struct v4l2_subdev *source_subdev = > + media_entity_to_v4l2_subdev(link->source->entity); > + int ret; > + > + /* Only support one enabled source at a time. */ > + if (bridge->source) { > + dev_err(dev, "multiple sources are connected to the bridge\n"); > + return -EBUSY; > + } > + > + ret = v4l2_subdev_link_validate(link); > + if (ret) > + return ret; > + > + if (source_subdev == bridge->source_parallel.subdev) > + bridge->source = &bridge->source_parallel; > + else Useless use of else. > + return -EINVAL; > + > + return 0; > +} > + > +static const struct media_entity_operations sun6i_csi_bridge_entity_ops = { > + .link_validate = sun6i_csi_bridge_link_validate, > +}; > + > +/* V4L2 Async */ > + > +static int sun6i_csi_bridge_link(struct sun6i_csi_device *csi_dev, > + int sink_pad_index, > + struct v4l2_subdev *remote_subdev, > + bool enabled) > +{ > + struct device *dev = csi_dev->dev; > + struct v4l2_subdev *subdev = &csi_dev->bridge.subdev; > + struct media_entity *sink_entity = &subdev->entity; > + struct media_entity *source_entity = &remote_subdev->entity; > + int source_pad_index; > + int ret; > + > + /* Get the first remote source pad. */ > + ret = media_entity_get_fwnode_pad(source_entity, remote_subdev->fwnode, > + MEDIA_PAD_FL_SOURCE); > + if (ret < 0) { > + dev_err(dev, "missing source pad in external entity %s\n", > + source_entity->name); > + return -EINVAL; > + } > + > + source_pad_index = ret; > + > + dev_dbg(dev, "creating %s:%u -> %s:%u link\n", source_entity->name, > + source_pad_index, sink_entity->name, sink_pad_index); > + > + ret = media_create_pad_link(source_entity, source_pad_index, > + sink_entity, sink_pad_index, > + enabled ? MEDIA_LNK_FL_ENABLED : 0); > + if (ret < 0) { > + dev_err(dev, "failed to create %s:%u -> %s:%u link\n", > + source_entity->name, source_pad_index, > + sink_entity->name, sink_pad_index); > + return ret; > + } > + > + return 0; > +} > + > +static int > +sun6i_csi_bridge_notifier_bound(struct v4l2_async_notifier *notifier, > + struct v4l2_subdev *remote_subdev, > + struct v4l2_async_subdev *async_subdev) > +{ > + struct sun6i_csi_device *csi_dev = > + container_of(notifier, struct sun6i_csi_device, > + bridge.notifier); > + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; > + struct sun6i_csi_bridge_source *source = NULL; > + struct fwnode_handle *fwnode = dev_fwnode(csi_dev->dev); > + struct fwnode_handle *handle = NULL; > + bool enabled; > + int ret; > + > + while ((handle = fwnode_graph_get_next_endpoint(fwnode, handle))) { I'd instead store the information you need here in struct sun6i_csi_bridge. You could remove the loop here. > + struct fwnode_endpoint endpoint = { 0 }; > + struct fwnode_handle *remote_fwnode; > + > + remote_fwnode = fwnode_graph_get_remote_port_parent(handle); > + if (!remote_fwnode) > + continue; > + > + if (remote_fwnode != remote_subdev->fwnode) > + goto next; > + > + ret = fwnode_graph_parse_endpoint(handle, &endpoint); > + if (ret < 0) > + goto next; > + > + switch (endpoint.port) { > + case SUN6I_CSI_PORT_PARALLEL: > + source = &bridge->source_parallel; > + enabled = true; > + break; > + default: > + break; > + } > + > +next: > + fwnode_handle_put(remote_fwnode); > + } > + > + if (!source) > + return -EINVAL; > + > + source->subdev = remote_subdev; > + > + return sun6i_csi_bridge_link(csi_dev, SUN6I_CSI_BRIDGE_PAD_SINK, > + remote_subdev, enabled); > +} > + > +static int > +sun6i_csi_bridge_notifier_complete(struct v4l2_async_notifier *notifier) > +{ > + struct sun6i_csi_device *csi_dev = > + container_of(notifier, struct sun6i_csi_device, > + bridge.notifier); > + > + return sun6i_csi_v4l2_complete(csi_dev); You could call v4l2_device_register_subdev_nodes() here. > +} > + > +static const struct v4l2_async_notifier_operations > +sun6i_csi_bridge_notifier_ops = { > + .bound = sun6i_csi_bridge_notifier_bound, > + .complete = sun6i_csi_bridge_notifier_complete, > +}; > + > +/* Bridge */ > + > +static int sun6i_csi_bridge_source_setup(struct sun6i_csi_device *csi_dev, > + struct sun6i_csi_bridge_source *source, > + u32 port, > + enum v4l2_mbus_type *bus_types) > +{ > + struct device *dev = csi_dev->dev; > + struct v4l2_async_notifier *notifier = &csi_dev->bridge.notifier; > + struct v4l2_fwnode_endpoint *endpoint = &source->endpoint; > + struct v4l2_async_subdev *async_subdev; > + struct fwnode_handle *handle; > + int ret; > + > + handle = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), port, 0, 0); > + if (!handle) > + return -ENODEV; > + > + ret = v4l2_fwnode_endpoint_parse(handle, endpoint); > + if (ret) > + goto complete; > + > + if (bus_types) { > + bool valid = false; > + unsigned int i; > + > + for (i = 0; bus_types[i] != V4L2_MBUS_INVALID; i++) { > + if (endpoint->bus_type == bus_types[i]) { > + valid = true; > + break; > + } > + } > + > + if (!valid) { > + dev_err(dev, "unsupported bus type for port %d\n", > + port); > + ret = -EINVAL; > + goto complete; > + } > + } > + > + async_subdev = v4l2_async_nf_add_fwnode_remote(notifier, handle, > + struct v4l2_async_subdev); > + if (IS_ERR(async_subdev)) { > + ret = PTR_ERR(async_subdev); > + goto complete; > + } > + > + source->expected = true; > + > +complete: > + fwnode_handle_put(handle); > + > + return ret; > +} > + > +int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev) > +{ > + struct device *dev = csi_dev->dev; > + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; > + struct v4l2_device *v4l2_dev = &csi_dev->v4l2.v4l2_dev; > + struct v4l2_subdev *subdev = &bridge->subdev; > + struct v4l2_async_notifier *notifier = &bridge->notifier; > + struct media_pad *pads = bridge->pads; > + enum v4l2_mbus_type parallel_mbus_types[] = { > + V4L2_MBUS_PARALLEL, > + V4L2_MBUS_BT656, > + V4L2_MBUS_INVALID > + }; > + int ret; > + > + /* V4L2 Subdev */ > + > + v4l2_subdev_init(subdev, &sun6i_csi_bridge_subdev_ops); > + strscpy(subdev->name, SUN6I_CSI_BRIDGE_NAME, sizeof(subdev->name)); > + subdev->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + subdev->owner = THIS_MODULE; > + subdev->dev = dev; > + > + v4l2_set_subdevdata(subdev, csi_dev); > + > + /* Media Entity */ > + > + subdev->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; > + subdev->entity.ops = &sun6i_csi_bridge_entity_ops; > + > + /* Media Pads */ > + > + pads[SUN6I_CSI_BRIDGE_PAD_SINK].flags = MEDIA_PAD_FL_SINK; > + pads[SUN6I_CSI_BRIDGE_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE | > + MEDIA_PAD_FL_MUST_CONNECT; > + > + ret = media_entity_pads_init(&subdev->entity, > + SUN6I_CSI_BRIDGE_PAD_COUNT, pads); > + if (ret < 0) > + return ret; > + > + /* V4L2 Subdev */ > + > + ret = v4l2_device_register_subdev(v4l2_dev, subdev); > + if (ret) { > + dev_err(dev, "failed to register v4l2 subdev: %d\n", ret); > + goto error_media_entity; > + } > + > + /* V4L2 Async */ > + > + v4l2_async_nf_init(notifier); > + notifier->ops = &sun6i_csi_bridge_notifier_ops; > + > + sun6i_csi_bridge_source_setup(csi_dev, &bridge->source_parallel, > + SUN6I_CSI_PORT_PARALLEL, > + parallel_mbus_types); > + > + ret = v4l2_async_nf_register(v4l2_dev, notifier); > + if (ret) { > + dev_err(dev, "failed to register v4l2 async notifier: %d\n", > + ret); > + goto error_v4l2_async_notifier; > + } > + > + return 0; > + > +error_v4l2_async_notifier: > + v4l2_async_nf_cleanup(notifier); > + > + v4l2_device_unregister_subdev(subdev); > + > +error_media_entity: > + media_entity_cleanup(&subdev->entity); > + > + return ret; > +} > + > +void sun6i_csi_bridge_cleanup(struct sun6i_csi_device *csi_dev) > +{ > + struct v4l2_subdev *subdev = &csi_dev->bridge.subdev; > + struct v4l2_async_notifier *notifier = &csi_dev->bridge.notifier; > + > + v4l2_async_nf_unregister(notifier); > + v4l2_async_nf_cleanup(notifier); > + > + v4l2_device_unregister_subdev(subdev); > + > + media_entity_cleanup(&subdev->entity); > +} > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h > new file mode 100644 > index 000000000000..2ee7878102b6 > --- /dev/null > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h > @@ -0,0 +1,44 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright 2021 Bootlin 2022? > + * Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > + */ > + > +#ifndef _SUN6I_CSI_BRIDGE_H_ > +#define _SUN6I_CSI_BRIDGE_H_ > + > +#include <media/v4l2-device.h> > +#include <media/v4l2-fwnode.h> > + > +#define SUN6I_CSI_BRIDGE_NAME "sun6i-csi-bridge" > + > +enum sun6i_csi_bridge_pad { > + SUN6I_CSI_BRIDGE_PAD_SINK = 0, > + SUN6I_CSI_BRIDGE_PAD_SOURCE = 1, > + SUN6I_CSI_BRIDGE_PAD_COUNT = 2, > +}; > + > +struct sun6i_csi_device; > + > +struct sun6i_csi_bridge_source { > + struct v4l2_subdev *subdev; > + struct v4l2_fwnode_endpoint endpoint; > + bool expected; > +}; > + > +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 sun6i_csi_bridge_source source_parallel; > + struct sun6i_csi_bridge_source *source; > +}; > + > +/* Bridge */ > + > +int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev); > +void sun6i_csi_bridge_cleanup(struct sun6i_csi_device *csi_dev); > + > +#endif > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c > index d32ff6b81f8a..fa5bf3697ace 100644 > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c > @@ -632,6 +632,7 @@ int sun6i_video_setup(struct sun6i_csi_device *csi_dev) > { > struct sun6i_video *video = &csi_dev->video; > struct v4l2_device *v4l2_dev = &csi_dev->v4l2.v4l2_dev; > + struct v4l2_subdev *bridge_subdev = &csi_dev->bridge.subdev; > struct video_device *video_dev = &video->video_dev; > struct vb2_queue *queue = &video->queue; > struct media_pad *pad = &video->pad; > @@ -715,8 +716,26 @@ int sun6i_video_setup(struct sun6i_csi_device *csi_dev) > v4l2_info(v4l2_dev, "device %s registered as %s\n", video_dev->name, > video_device_node_name(video_dev)); > > + /* Media Pad Link */ > + > + ret = media_create_pad_link(&bridge_subdev->entity, > + SUN6I_CSI_BRIDGE_PAD_SOURCE, > + &video_dev->entity, 0, > + MEDIA_LNK_FL_ENABLED | > + MEDIA_LNK_FL_IMMUTABLE); > + if (ret < 0) { > + v4l2_err(v4l2_dev, "failed to create %s:%u -> %s:%u link\n", > + bridge_subdev->entity.name, > + SUN6I_CSI_BRIDGE_PAD_SOURCE, > + video_dev->entity.name, 0); > + goto error_video_device; > + } > + > return 0; > > +error_video_device: > + vb2_video_unregister_device(video_dev); > + > error_media_entity: > media_entity_cleanup(&video_dev->entity); >
Hi Sakari, On Mon 14 Feb 22, 20:12, Sakari Ailus wrote: > Hi Paul, > > On Sat, Feb 05, 2022 at 07:53:53PM +0100, Paul Kocialkowski wrote: > > Introduce a bridge v4l2 subdev to prepare for separation between the > > processing part (bridge) and the dma engine, which is required to > > properly support ths isp workflow later on. > > > > Currently the bridge just manages fwnode mapping to media pads, > > using an async notifier (which was previously in the main code). > > The s_stream video op just forwards to the connected v4l2 subdev > > (sensor or MIPI CSI-2 bridge). > > > > The video capture device is now registered after the bridge and > > attaches to it with a media link. > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > --- > > .../media/platform/sunxi/sun6i-csi/Makefile | 2 +- > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 156 +----- > > .../platform/sunxi/sun6i-csi/sun6i_csi.h | 12 +- > > .../sunxi/sun6i-csi/sun6i_csi_bridge.c | 473 ++++++++++++++++++ > > .../sunxi/sun6i-csi/sun6i_csi_bridge.h | 44 ++ > > .../platform/sunxi/sun6i-csi/sun6i_video.c | 19 + > > 6 files changed, 571 insertions(+), 135 deletions(-) > > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c > > create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h [...] > > +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; > > + > > + sun6i_csi_bridge_mbus_format_prepare(mbus_format); > > + > > + if (format->which == V4L2_SUBDEV_FORMAT_TRY) > > + *v4l2_subdev_get_try_format(subdev, state, format->pad) = > > + *mbus_format; > > + else > > + csi_dev->bridge.mbus_format = *mbus_format; > > Note that the driver is responsible for serialising access to its data, > i.e. you have to acquire the mutex here. Thanks, will take care of that next time. > > + > > + return 0; > > +} > > + > > +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, > > + .set_fmt = sun6i_csi_bridge_set_fmt, > > +}; > > + > > +const struct v4l2_subdev_ops sun6i_csi_bridge_subdev_ops = { > > + .video = &sun6i_csi_bridge_video_ops, > > + .pad = &sun6i_csi_bridge_pad_ops, > > +}; > > + > > +/* Media Entity */ > > + > > +static int sun6i_csi_bridge_link_validate(struct media_link *link) > > +{ > > + struct v4l2_subdev *subdev = > > + media_entity_to_v4l2_subdev(link->sink->entity); > > + struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev); > > + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; > > + struct device *dev = csi_dev->dev; > > + struct v4l2_subdev *source_subdev = > > + media_entity_to_v4l2_subdev(link->source->entity); > > + int ret; > > + > > + /* Only support one enabled source at a time. */ > > + if (bridge->source) { > > + dev_err(dev, "multiple sources are connected to the bridge\n"); > > + return -EBUSY; > > + } > > + > > + ret = v4l2_subdev_link_validate(link); > > + if (ret) > > + return ret; > > + > > + if (source_subdev == bridge->source_parallel.subdev) > > + bridge->source = &bridge->source_parallel; > > + else > > Useless use of else. Fair enough. > > + return -EINVAL; > > + > > + return 0; > > +} > > + > > +static const struct media_entity_operations sun6i_csi_bridge_entity_ops = { > > + .link_validate = sun6i_csi_bridge_link_validate, > > +}; > > + > > +/* V4L2 Async */ > > + > > +static int sun6i_csi_bridge_link(struct sun6i_csi_device *csi_dev, > > + int sink_pad_index, > > + struct v4l2_subdev *remote_subdev, > > + bool enabled) > > +{ > > + struct device *dev = csi_dev->dev; > > + struct v4l2_subdev *subdev = &csi_dev->bridge.subdev; > > + struct media_entity *sink_entity = &subdev->entity; > > + struct media_entity *source_entity = &remote_subdev->entity; > > + int source_pad_index; > > + int ret; > > + > > + /* Get the first remote source pad. */ > > + ret = media_entity_get_fwnode_pad(source_entity, remote_subdev->fwnode, > > + MEDIA_PAD_FL_SOURCE); > > + if (ret < 0) { > > + dev_err(dev, "missing source pad in external entity %s\n", > > + source_entity->name); > > + return -EINVAL; > > + } > > + > > + source_pad_index = ret; > > + > > + dev_dbg(dev, "creating %s:%u -> %s:%u link\n", source_entity->name, > > + source_pad_index, sink_entity->name, sink_pad_index); > > + > > + ret = media_create_pad_link(source_entity, source_pad_index, > > + sink_entity, sink_pad_index, > > + enabled ? MEDIA_LNK_FL_ENABLED : 0); > > + if (ret < 0) { > > + dev_err(dev, "failed to create %s:%u -> %s:%u link\n", > > + source_entity->name, source_pad_index, > > + sink_entity->name, sink_pad_index); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int > > +sun6i_csi_bridge_notifier_bound(struct v4l2_async_notifier *notifier, > > + struct v4l2_subdev *remote_subdev, > > + struct v4l2_async_subdev *async_subdev) > > +{ > > + struct sun6i_csi_device *csi_dev = > > + container_of(notifier, struct sun6i_csi_device, > > + bridge.notifier); > > + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; > > + struct sun6i_csi_bridge_source *source = NULL; > > + struct fwnode_handle *fwnode = dev_fwnode(csi_dev->dev); > > + struct fwnode_handle *handle = NULL; > > + bool enabled; > > + int ret; > > + > > + while ((handle = fwnode_graph_get_next_endpoint(fwnode, handle))) { > > I'd instead store the information you need here in struct sun6i_csi_bridge. > You could remove the loop here. Is there a different method for matching a remote subdev to a local port? The rationale here is that I need the handle for fwnode_graph_parse_endpoint but cannot get that handle from the remote subdev's fwnode pointer directly. > > + struct fwnode_endpoint endpoint = { 0 }; > > + struct fwnode_handle *remote_fwnode; > > + > > + remote_fwnode = fwnode_graph_get_remote_port_parent(handle); > > + if (!remote_fwnode) > > + continue; > > + > > + if (remote_fwnode != remote_subdev->fwnode) > > + goto next; > > + > > + ret = fwnode_graph_parse_endpoint(handle, &endpoint); > > + if (ret < 0) > > + goto next; > > + > > + switch (endpoint.port) { > > + case SUN6I_CSI_PORT_PARALLEL: > > + source = &bridge->source_parallel; > > + enabled = true; > > + break; > > + default: > > + break; > > + } > > + > > +next: > > + fwnode_handle_put(remote_fwnode); > > + } > > + > > + if (!source) > > + return -EINVAL; > > + > > + source->subdev = remote_subdev; > > + > > + return sun6i_csi_bridge_link(csi_dev, SUN6I_CSI_BRIDGE_PAD_SINK, > > + remote_subdev, enabled); > > +} > > + > > +static int > > +sun6i_csi_bridge_notifier_complete(struct v4l2_async_notifier *notifier) > > +{ > > + struct sun6i_csi_device *csi_dev = > > + container_of(notifier, struct sun6i_csi_device, > > + bridge.notifier); > > + > > + return sun6i_csi_v4l2_complete(csi_dev); > > You could call v4l2_device_register_subdev_nodes() here. That's definitely what sun6i_csi_v4l2_complete does (the diff is probably not very clear). Note that the wrapper is extended later on to register the capture video device for the no-isp path. Maybe the capture registration could be kept in sun6i_csi_probe for the non-isp path and then the wrapper wouldn't be needed. I don't mind either way. > > +} > > + > > +static const struct v4l2_async_notifier_operations > > +sun6i_csi_bridge_notifier_ops = { > > + .bound = sun6i_csi_bridge_notifier_bound, > > + .complete = sun6i_csi_bridge_notifier_complete, > > +}; > > + > > +/* Bridge */ > > + > > +static int sun6i_csi_bridge_source_setup(struct sun6i_csi_device *csi_dev, > > + struct sun6i_csi_bridge_source *source, > > + u32 port, > > + enum v4l2_mbus_type *bus_types) > > +{ > > + struct device *dev = csi_dev->dev; > > + struct v4l2_async_notifier *notifier = &csi_dev->bridge.notifier; > > + struct v4l2_fwnode_endpoint *endpoint = &source->endpoint; > > + struct v4l2_async_subdev *async_subdev; > > + struct fwnode_handle *handle; > > + int ret; > > + > > + handle = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), port, 0, 0); > > + if (!handle) > > + return -ENODEV; > > + > > + ret = v4l2_fwnode_endpoint_parse(handle, endpoint); > > + if (ret) > > + goto complete; > > + > > + if (bus_types) { > > + bool valid = false; > > + unsigned int i; > > + > > + for (i = 0; bus_types[i] != V4L2_MBUS_INVALID; i++) { > > + if (endpoint->bus_type == bus_types[i]) { > > + valid = true; > > + break; > > + } > > + } > > + > > + if (!valid) { > > + dev_err(dev, "unsupported bus type for port %d\n", > > + port); > > + ret = -EINVAL; > > + goto complete; > > + } > > + } > > + > > + async_subdev = v4l2_async_nf_add_fwnode_remote(notifier, handle, > > + struct v4l2_async_subdev); > > + if (IS_ERR(async_subdev)) { > > + ret = PTR_ERR(async_subdev); > > + goto complete; > > + } > > + > > + source->expected = true; > > + > > +complete: > > + fwnode_handle_put(handle); > > + > > + return ret; > > +} > > + > > +int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev) > > +{ > > + struct device *dev = csi_dev->dev; > > + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; > > + struct v4l2_device *v4l2_dev = &csi_dev->v4l2.v4l2_dev; > > + struct v4l2_subdev *subdev = &bridge->subdev; > > + struct v4l2_async_notifier *notifier = &bridge->notifier; > > + struct media_pad *pads = bridge->pads; > > + enum v4l2_mbus_type parallel_mbus_types[] = { > > + V4L2_MBUS_PARALLEL, > > + V4L2_MBUS_BT656, > > + V4L2_MBUS_INVALID > > + }; > > + int ret; > > + > > + /* V4L2 Subdev */ > > + > > + v4l2_subdev_init(subdev, &sun6i_csi_bridge_subdev_ops); > > + strscpy(subdev->name, SUN6I_CSI_BRIDGE_NAME, sizeof(subdev->name)); > > + subdev->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > + subdev->owner = THIS_MODULE; > > + subdev->dev = dev; > > + > > + v4l2_set_subdevdata(subdev, csi_dev); > > + > > + /* Media Entity */ > > + > > + subdev->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; > > + subdev->entity.ops = &sun6i_csi_bridge_entity_ops; > > + > > + /* Media Pads */ > > + > > + pads[SUN6I_CSI_BRIDGE_PAD_SINK].flags = MEDIA_PAD_FL_SINK; > > + pads[SUN6I_CSI_BRIDGE_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE | > > + MEDIA_PAD_FL_MUST_CONNECT; > > + > > + ret = media_entity_pads_init(&subdev->entity, > > + SUN6I_CSI_BRIDGE_PAD_COUNT, pads); > > + if (ret < 0) > > + return ret; > > + > > + /* V4L2 Subdev */ > > + > > + ret = v4l2_device_register_subdev(v4l2_dev, subdev); > > + if (ret) { > > + dev_err(dev, "failed to register v4l2 subdev: %d\n", ret); > > + goto error_media_entity; > > + } > > + > > + /* V4L2 Async */ > > + > > + v4l2_async_nf_init(notifier); > > + notifier->ops = &sun6i_csi_bridge_notifier_ops; > > + > > + sun6i_csi_bridge_source_setup(csi_dev, &bridge->source_parallel, > > + SUN6I_CSI_PORT_PARALLEL, > > + parallel_mbus_types); > > + > > + ret = v4l2_async_nf_register(v4l2_dev, notifier); > > + if (ret) { > > + dev_err(dev, "failed to register v4l2 async notifier: %d\n", > > + ret); > > + goto error_v4l2_async_notifier; > > + } > > + > > + return 0; > > + > > +error_v4l2_async_notifier: > > + v4l2_async_nf_cleanup(notifier); > > + > > + v4l2_device_unregister_subdev(subdev); > > + > > +error_media_entity: > > + media_entity_cleanup(&subdev->entity); > > + > > + return ret; > > +} > > + > > +void sun6i_csi_bridge_cleanup(struct sun6i_csi_device *csi_dev) > > +{ > > + struct v4l2_subdev *subdev = &csi_dev->bridge.subdev; > > + struct v4l2_async_notifier *notifier = &csi_dev->bridge.notifier; > > + > > + v4l2_async_nf_unregister(notifier); > > + v4l2_async_nf_cleanup(notifier); > > + > > + v4l2_device_unregister_subdev(subdev); > > + > > + media_entity_cleanup(&subdev->entity); > > +} > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h > > new file mode 100644 > > index 000000000000..2ee7878102b6 > > --- /dev/null > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h > > @@ -0,0 +1,44 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > +/* > > + * Copyright 2021 Bootlin > > 2022? Right, thanks! > > + * Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > + */ > > + > > +#ifndef _SUN6I_CSI_BRIDGE_H_ > > +#define _SUN6I_CSI_BRIDGE_H_ > > + > > +#include <media/v4l2-device.h> > > +#include <media/v4l2-fwnode.h> > > + > > +#define SUN6I_CSI_BRIDGE_NAME "sun6i-csi-bridge" > > + > > +enum sun6i_csi_bridge_pad { > > + SUN6I_CSI_BRIDGE_PAD_SINK = 0, > > + SUN6I_CSI_BRIDGE_PAD_SOURCE = 1, > > + SUN6I_CSI_BRIDGE_PAD_COUNT = 2, > > +}; > > + > > +struct sun6i_csi_device; > > + > > +struct sun6i_csi_bridge_source { > > + struct v4l2_subdev *subdev; > > + struct v4l2_fwnode_endpoint endpoint; > > + bool expected; > > +}; > > + > > +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 sun6i_csi_bridge_source source_parallel; > > + struct sun6i_csi_bridge_source *source; > > +}; > > + > > +/* Bridge */ > > + > > +int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev); > > +void sun6i_csi_bridge_cleanup(struct sun6i_csi_device *csi_dev); > > + > > +#endif > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c > > index d32ff6b81f8a..fa5bf3697ace 100644 > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c > > @@ -632,6 +632,7 @@ int sun6i_video_setup(struct sun6i_csi_device *csi_dev) > > { > > struct sun6i_video *video = &csi_dev->video; > > struct v4l2_device *v4l2_dev = &csi_dev->v4l2.v4l2_dev; > > + struct v4l2_subdev *bridge_subdev = &csi_dev->bridge.subdev; > > struct video_device *video_dev = &video->video_dev; > > struct vb2_queue *queue = &video->queue; > > struct media_pad *pad = &video->pad; > > @@ -715,8 +716,26 @@ int sun6i_video_setup(struct sun6i_csi_device *csi_dev) > > v4l2_info(v4l2_dev, "device %s registered as %s\n", video_dev->name, > > video_device_node_name(video_dev)); > > > > + /* Media Pad Link */ > > + > > + ret = media_create_pad_link(&bridge_subdev->entity, > > + SUN6I_CSI_BRIDGE_PAD_SOURCE, > > + &video_dev->entity, 0, > > + MEDIA_LNK_FL_ENABLED | > > + MEDIA_LNK_FL_IMMUTABLE); > > + if (ret < 0) { > > + v4l2_err(v4l2_dev, "failed to create %s:%u -> %s:%u link\n", > > + bridge_subdev->entity.name, > > + SUN6I_CSI_BRIDGE_PAD_SOURCE, > > + video_dev->entity.name, 0); > > + goto error_video_device; > > + } > > + > > return 0; > > > > +error_video_device: > > + vb2_video_unregister_device(video_dev); > > + > > error_media_entity: > > media_entity_cleanup(&video_dev->entity); > > > > -- > Sakari Ailus
Hi Paul, On Wed, Mar 02, 2022 at 03:59:50PM +0100, Paul Kocialkowski wrote: > > > +static int > > > +sun6i_csi_bridge_notifier_bound(struct v4l2_async_notifier *notifier, > > > + struct v4l2_subdev *remote_subdev, > > > + struct v4l2_async_subdev *async_subdev) > > > +{ > > > + struct sun6i_csi_device *csi_dev = > > > + container_of(notifier, struct sun6i_csi_device, > > > + bridge.notifier); > > > + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; > > > + struct sun6i_csi_bridge_source *source = NULL; > > > + struct fwnode_handle *fwnode = dev_fwnode(csi_dev->dev); > > > + struct fwnode_handle *handle = NULL; > > > + bool enabled; > > > + int ret; > > > + > > > + while ((handle = fwnode_graph_get_next_endpoint(fwnode, handle))) { > > > > I'd instead store the information you need here in struct sun6i_csi_bridge. > > You could remove the loop here. > > Is there a different method for matching a remote subdev to a local port? > The rationale here is that I need the handle for fwnode_graph_parse_endpoint > but cannot get that handle from the remote subdev's fwnode pointer directly. You generally shouldn't try to match fwnodes here as the V4L2 async framework has already done that job. This information can be found behind the async_subdev pointer. See e.g. drivers/media/pci/intel/ipu3/ipu3-cio2-main.c for an example. > > > > + struct fwnode_endpoint endpoint = { 0 }; > > > + struct fwnode_handle *remote_fwnode; > > > + > > > + remote_fwnode = fwnode_graph_get_remote_port_parent(handle); > > > + if (!remote_fwnode) > > > + continue; > > > + > > > + if (remote_fwnode != remote_subdev->fwnode) > > > + goto next; > > > + > > > + ret = fwnode_graph_parse_endpoint(handle, &endpoint); > > > + if (ret < 0) > > > + goto next; > > > + > > > + switch (endpoint.port) { > > > + case SUN6I_CSI_PORT_PARALLEL: > > > + source = &bridge->source_parallel; > > > + enabled = true; > > > + break; > > > + default: > > > + break; > > > + } > > > + > > > +next: > > > + fwnode_handle_put(remote_fwnode); > > > + } > > > + > > > + if (!source) > > > + return -EINVAL; > > > + > > > + source->subdev = remote_subdev; > > > + > > > + return sun6i_csi_bridge_link(csi_dev, SUN6I_CSI_BRIDGE_PAD_SINK, > > > + remote_subdev, enabled); > > > +} > > > + > > > +static int > > > +sun6i_csi_bridge_notifier_complete(struct v4l2_async_notifier *notifier) > > > +{ > > > + struct sun6i_csi_device *csi_dev = > > > + container_of(notifier, struct sun6i_csi_device, > > > + bridge.notifier); > > > + > > > + return sun6i_csi_v4l2_complete(csi_dev); > > > > You could call v4l2_device_register_subdev_nodes() here. > > That's definitely what sun6i_csi_v4l2_complete does (the diff is probably not > very clear). Note that the wrapper is extended later on to register the capture > video device for the no-isp path. I could be missing something... Do you need to call sun6i_csi_v4l2_complete() in multiple places or not? If not, then I think it'd be probably better to just move the code here. > > Maybe the capture registration could be kept in sun6i_csi_probe for the non-isp > path and then the wrapper wouldn't be needed. I don't mind either way.
Hi Sakari, On Fri 04 Mar 22, 00:43, Sakari Ailus wrote: > Hi Paul, > > On Wed, Mar 02, 2022 at 03:59:50PM +0100, Paul Kocialkowski wrote: > > > > +static int > > > > +sun6i_csi_bridge_notifier_bound(struct v4l2_async_notifier *notifier, > > > > + struct v4l2_subdev *remote_subdev, > > > > + struct v4l2_async_subdev *async_subdev) > > > > +{ > > > > + struct sun6i_csi_device *csi_dev = > > > > + container_of(notifier, struct sun6i_csi_device, > > > > + bridge.notifier); > > > > + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; > > > > + struct sun6i_csi_bridge_source *source = NULL; > > > > + struct fwnode_handle *fwnode = dev_fwnode(csi_dev->dev); > > > > + struct fwnode_handle *handle = NULL; > > > > + bool enabled; > > > > + int ret; > > > > + > > > > + while ((handle = fwnode_graph_get_next_endpoint(fwnode, handle))) { > > > > > > I'd instead store the information you need here in struct sun6i_csi_bridge. > > > You could remove the loop here. > > > > Is there a different method for matching a remote subdev to a local port? > > The rationale here is that I need the handle for fwnode_graph_parse_endpoint > > but cannot get that handle from the remote subdev's fwnode pointer directly. > > You generally shouldn't try to match fwnodes here as the V4L2 async > framework has already done that job. This information can be found behind > the async_subdev pointer. > > See e.g. drivers/media/pci/intel/ipu3/ipu3-cio2-main.c for an example. Thanks for the feedback, I'll look into that. > > > > > > + struct fwnode_endpoint endpoint = { 0 }; > > > > + struct fwnode_handle *remote_fwnode; > > > > + > > > > + remote_fwnode = fwnode_graph_get_remote_port_parent(handle); > > > > + if (!remote_fwnode) > > > > + continue; > > > > + > > > > + if (remote_fwnode != remote_subdev->fwnode) > > > > + goto next; > > > > + > > > > + ret = fwnode_graph_parse_endpoint(handle, &endpoint); > > > > + if (ret < 0) > > > > + goto next; > > > > + > > > > + switch (endpoint.port) { > > > > + case SUN6I_CSI_PORT_PARALLEL: > > > > + source = &bridge->source_parallel; > > > > + enabled = true; > > > > + break; > > > > + default: > > > > + break; > > > > + } > > > > + > > > > +next: > > > > + fwnode_handle_put(remote_fwnode); > > > > + } > > > > + > > > > + if (!source) > > > > + return -EINVAL; > > > > + > > > > + source->subdev = remote_subdev; > > > > + > > > > + return sun6i_csi_bridge_link(csi_dev, SUN6I_CSI_BRIDGE_PAD_SINK, > > > > + remote_subdev, enabled); > > > > +} > > > > + > > > > +static int > > > > +sun6i_csi_bridge_notifier_complete(struct v4l2_async_notifier *notifier) > > > > +{ > > > > + struct sun6i_csi_device *csi_dev = > > > > + container_of(notifier, struct sun6i_csi_device, > > > > + bridge.notifier); > > > > + > > > > + return sun6i_csi_v4l2_complete(csi_dev); > > > > > > You could call v4l2_device_register_subdev_nodes() here. > > > > That's definitely what sun6i_csi_v4l2_complete does (the diff is probably not > > very clear). Note that the wrapper is extended later on to register the capture > > video device for the no-isp path. > > I could be missing something... Do you need to call > sun6i_csi_v4l2_complete() in multiple places or not? If not, then I think > it'd be probably better to just move the code here. No this is only called here so I guess we can avoid it entirely. Thanks, Paul > > > > Maybe the capture registration could be kept in sun6i_csi_probe for the non-isp > > path and then the wrapper wouldn't be needed. I don't mind either way. > > -- > Kind regards, > > Sakari Ailus
diff --git a/drivers/media/platform/sunxi/sun6i-csi/Makefile b/drivers/media/platform/sunxi/sun6i-csi/Makefile index e7e315347804..7a699580a641 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/Makefile +++ b/drivers/media/platform/sunxi/sun6i-csi/Makefile @@ -1,4 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only -sun6i-csi-y += sun6i_video.o sun6i_csi.o +sun6i-csi-y += sun6i_video.o sun6i_csi.o sun6i_csi_bridge.o obj-$(CONFIG_VIDEO_SUN6I_CSI) += sun6i-csi.o diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c index c8fe31cc38b5..a1847ae3e88e 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c @@ -34,16 +34,17 @@ bool sun6i_csi_is_format_supported(struct sun6i_csi_device *csi_dev, u32 pixformat, u32 mbus_code) { - struct sun6i_csi_v4l2 *v4l2 = &csi_dev->v4l2; + struct v4l2_fwnode_endpoint *endpoint = + &csi_dev->bridge.source->endpoint; /* * Some video receivers have the ability to be compatible with * 8bit and 16bit bus width. * Identify the media bus format from device tree. */ - if ((v4l2->v4l2_ep.bus_type == V4L2_MBUS_PARALLEL - || v4l2->v4l2_ep.bus_type == V4L2_MBUS_BT656) - && v4l2->v4l2_ep.bus.parallel.bus_width == 16) { + if ((endpoint->bus_type == V4L2_MBUS_PARALLEL + || endpoint->bus_type == V4L2_MBUS_BT656) + && endpoint->bus.parallel.bus_width == 16) { switch (pixformat) { case V4L2_PIX_FMT_NV12_16L16: case V4L2_PIX_FMT_NV12: @@ -328,7 +329,8 @@ static enum csi_input_seq get_csi_input_seq(struct sun6i_csi_device *csi_dev, static void sun6i_csi_setup_bus(struct sun6i_csi_device *csi_dev) { - struct v4l2_fwnode_endpoint *endpoint = &csi_dev->v4l2.v4l2_ep; + struct v4l2_fwnode_endpoint *endpoint = + &csi_dev->bridge.source->endpoint; struct sun6i_csi_config *config = &csi_dev->config; unsigned char bus_width; u32 flags; @@ -583,95 +585,11 @@ static const struct media_device_ops sun6i_csi_media_ops = { /* V4L2 */ -static int sun6i_csi_link_entity(struct sun6i_csi_device *csi_dev, - struct media_entity *entity, - struct fwnode_handle *fwnode) +int sun6i_csi_v4l2_complete(struct sun6i_csi_device *csi_dev) { - struct media_entity *sink; - struct media_pad *sink_pad; - int src_pad_index; - int ret; - - ret = media_entity_get_fwnode_pad(entity, fwnode, MEDIA_PAD_FL_SOURCE); - if (ret < 0) { - dev_err(csi_dev->dev, - "%s: no source pad in external entity %s\n", __func__, - entity->name); - return -EINVAL; - } - - src_pad_index = ret; - - sink = &csi_dev->video.video_dev.entity; - sink_pad = &csi_dev->video.pad; - - dev_dbg(csi_dev->dev, "creating %s:%u -> %s:%u link\n", - entity->name, src_pad_index, sink->name, sink_pad->index); - ret = media_create_pad_link(entity, src_pad_index, sink, - sink_pad->index, - MEDIA_LNK_FL_ENABLED | - MEDIA_LNK_FL_IMMUTABLE); - if (ret < 0) { - dev_err(csi_dev->dev, "failed to create %s:%u -> %s:%u link\n", - entity->name, src_pad_index, - sink->name, sink_pad->index); - return ret; - } - - return 0; -} - -static int sun6i_subdev_notify_complete(struct v4l2_async_notifier *notifier) -{ - struct sun6i_csi_device *csi_dev = - container_of(notifier, struct sun6i_csi_device, - v4l2.notifier); - struct sun6i_csi_v4l2 *v4l2 = &csi_dev->v4l2; - struct v4l2_device *v4l2_dev = &v4l2->v4l2_dev; - struct v4l2_subdev *sd; - int ret; - - dev_dbg(csi_dev->dev, "notify complete, all subdevs registered\n"); - - sd = list_first_entry(&v4l2_dev->subdevs, struct v4l2_subdev, list); - if (!sd) - return -EINVAL; - - ret = sun6i_csi_link_entity(csi_dev, &sd->entity, sd->fwnode); - if (ret < 0) - return ret; - - ret = v4l2_device_register_subdev_nodes(v4l2_dev); - if (ret < 0) - return ret; - - return 0; -} - -static const struct v4l2_async_notifier_operations sun6i_csi_async_ops = { - .complete = sun6i_subdev_notify_complete, -}; - -static int sun6i_csi_fwnode_parse(struct device *dev, - struct v4l2_fwnode_endpoint *vep, - struct v4l2_async_subdev *asd) -{ - struct sun6i_csi_device *csi_dev = dev_get_drvdata(dev); + struct v4l2_device *v4l2_dev = &csi_dev->v4l2.v4l2_dev; - if (vep->base.port || vep->base.id) { - dev_warn(dev, "Only support a single port with one endpoint\n"); - return -ENOTCONN; - } - - switch (vep->bus_type) { - case V4L2_MBUS_PARALLEL: - case V4L2_MBUS_BT656: - csi_dev->v4l2.v4l2_ep = *vep; - return 0; - default: - dev_err(dev, "Unsupported media bus type\n"); - return -ENOTCONN; - } + return v4l2_device_register_subdev_nodes(v4l2_dev); } static int sun6i_csi_v4l2_setup(struct sun6i_csi_device *csi_dev) @@ -679,7 +597,6 @@ static int sun6i_csi_v4l2_setup(struct sun6i_csi_device *csi_dev) struct sun6i_csi_v4l2 *v4l2 = &csi_dev->v4l2; struct media_device *media_dev = &v4l2->media_dev; struct v4l2_device *v4l2_dev = &v4l2->v4l2_dev; - struct v4l2_async_notifier *notifier = &v4l2->notifier; struct device *dev = csi_dev->dev; int ret; @@ -720,42 +637,8 @@ static int sun6i_csi_v4l2_setup(struct sun6i_csi_device *csi_dev) goto error_v4l2_ctrl; } - /* Video */ - - ret = sun6i_video_setup(csi_dev); - if (ret) - goto error_v4l2_device; - - /* V4L2 Async */ - - v4l2_async_nf_init(notifier); - notifier->ops = &sun6i_csi_async_ops; - - ret = v4l2_async_nf_parse_fwnode_endpoints(dev, notifier, - sizeof(struct - v4l2_async_subdev), - sun6i_csi_fwnode_parse); - if (ret) - goto error_video; - - ret = v4l2_async_nf_register(v4l2_dev, notifier); - if (ret) { - dev_err(dev, "failed to register v4l2 async notifier: %d\n", - ret); - goto error_v4l2_async_notifier; - } - return 0; -error_v4l2_async_notifier: - v4l2_async_nf_cleanup(notifier); - -error_video: - sun6i_video_cleanup(csi_dev); - -error_v4l2_device: - v4l2_device_unregister(&v4l2->v4l2_dev); - error_v4l2_ctrl: v4l2_ctrl_handler_free(&v4l2->ctrl_handler); @@ -771,9 +654,6 @@ static void sun6i_csi_v4l2_cleanup(struct sun6i_csi_device *csi_dev) struct sun6i_csi_v4l2 *v4l2 = &csi_dev->v4l2; media_device_unregister(&v4l2->media_dev); - v4l2_async_nf_unregister(&v4l2->notifier); - v4l2_async_nf_cleanup(&v4l2->notifier); - sun6i_video_cleanup(csi_dev); v4l2_device_unregister(&v4l2->v4l2_dev); v4l2_ctrl_handler_free(&v4l2->ctrl_handler); media_device_cleanup(&v4l2->media_dev); @@ -995,8 +875,22 @@ static int sun6i_csi_probe(struct platform_device *platform_dev) if (ret) goto error_resources; + ret = sun6i_csi_bridge_setup(csi_dev); + if (ret) + goto error_v4l2; + + ret = sun6i_video_setup(csi_dev); + if (ret) + goto error_bridge; + return 0; +error_bridge: + sun6i_csi_bridge_cleanup(csi_dev); + +error_v4l2: + sun6i_csi_v4l2_cleanup(csi_dev); + error_resources: sun6i_csi_resources_cleanup(csi_dev); @@ -1007,6 +901,8 @@ static int sun6i_csi_remove(struct platform_device *pdev) { struct sun6i_csi_device *csi_dev = platform_get_drvdata(pdev); + sun6i_video_cleanup(csi_dev); + sun6i_csi_bridge_cleanup(csi_dev); sun6i_csi_v4l2_cleanup(csi_dev); sun6i_csi_resources_cleanup(csi_dev); diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h index 4dd83e57bafa..576c7f10289e 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h @@ -13,11 +13,16 @@ #include <media/v4l2-fwnode.h> #include <media/videobuf2-v4l2.h> +#include "sun6i_csi_bridge.h" #include "sun6i_video.h" #define SUN6I_CSI_NAME "sun6i-csi" #define SUN6I_CSI_DESCRIPTION "Allwinner A31 CSI Device" +enum sun6i_csi_port { + SUN6I_CSI_PORT_PARALLEL = 0, +}; + struct sun6i_csi_buffer { struct vb2_v4l2_buffer v4l2_buffer; struct list_head list; @@ -46,10 +51,6 @@ struct sun6i_csi_v4l2 { struct v4l2_device v4l2_dev; struct v4l2_ctrl_handler ctrl_handler; struct media_device media_dev; - - struct v4l2_async_notifier notifier; - /* video port settings */ - struct v4l2_fwnode_endpoint v4l2_ep; }; struct sun6i_csi_device { @@ -57,6 +58,7 @@ struct sun6i_csi_device { struct sun6i_csi_config config; struct sun6i_csi_v4l2 v4l2; + struct sun6i_csi_bridge bridge; struct sun6i_video video; struct regmap *regmap; @@ -156,4 +158,6 @@ static inline int sun6i_csi_get_bpp(unsigned int pixformat) return 0; } +int sun6i_csi_v4l2_complete(struct sun6i_csi_device *csi_dev); + #endif /* __SUN6I_CSI_H__ */ diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c new file mode 100644 index 000000000000..74706d883359 --- /dev/null +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c @@ -0,0 +1,473 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2021-2022 Bootlin + * Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com> + */ + +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <media/v4l2-device.h> +#include <media/v4l2-fwnode.h> + +#include "sun6i_csi.h" +#include "sun6i_csi_bridge.h" + +/* Format */ + +static const u32 sun6i_csi_bridge_mbus_codes[] = { + /* Bayer */ + MEDIA_BUS_FMT_SBGGR8_1X8, + MEDIA_BUS_FMT_SGBRG8_1X8, + MEDIA_BUS_FMT_SGRBG8_1X8, + MEDIA_BUS_FMT_SRGGB8_1X8, + MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10, + MEDIA_BUS_FMT_SBGGR12_1X12, + MEDIA_BUS_FMT_SGBRG12_1X12, + MEDIA_BUS_FMT_SGRBG12_1X12, + MEDIA_BUS_FMT_SRGGB12_1X12, + /* RGB */ + MEDIA_BUS_FMT_RGB565_2X8_LE, + MEDIA_BUS_FMT_RGB565_2X8_BE, + /* YUV422 */ + MEDIA_BUS_FMT_YUYV8_2X8, + MEDIA_BUS_FMT_UYVY8_2X8, + MEDIA_BUS_FMT_YVYU8_2X8, + MEDIA_BUS_FMT_UYVY8_2X8, + MEDIA_BUS_FMT_VYUY8_2X8, + MEDIA_BUS_FMT_YUYV8_1X16, + MEDIA_BUS_FMT_UYVY8_1X16, + MEDIA_BUS_FMT_YVYU8_1X16, + MEDIA_BUS_FMT_UYVY8_1X16, + MEDIA_BUS_FMT_VYUY8_1X16, + /* Compressed */ + MEDIA_BUS_FMT_JPEG_1X8, +}; + +static bool sun6i_csi_bridge_mbus_code_check(u32 mbus_code) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(sun6i_csi_bridge_mbus_codes); i++) + if (sun6i_csi_bridge_mbus_codes[i] == mbus_code) + return true; + + return false; +} + +/* V4L2 Subdev */ + +static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on) +{ + struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev); + struct v4l2_subdev *source_subdev; + /* Initialize to 0 to use both in disable label (ret != 0) and off. */ + int ret = 0; + + /* Source */ + + if (!csi_dev->bridge.source) + return -ENODEV; + + source_subdev = csi_dev->bridge.source->subdev; + + if (!on) { + v4l2_subdev_call(source_subdev, video, s_stream, 0); + goto disable; + } + + ret = v4l2_subdev_call(source_subdev, video, s_stream, 1); + if (ret && ret != -ENOIOCTLCMD) + goto disable; + + return 0; + +disable: + csi_dev->bridge.source = NULL; + + return ret; +} + +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_mbus_code_check(mbus_format->code)) + mbus_format->code = sun6i_csi_bridge_mbus_codes[0]; + + 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) +{ + unsigned int pad = SUN6I_CSI_BRIDGE_PAD_SINK; + struct v4l2_mbus_framefmt *mbus_format = + v4l2_subdev_get_try_format(subdev, state, pad); + + mbus_format->code = sun6i_csi_bridge_mbus_codes[0]; + mbus_format->width = 1280; + mbus_format->height = 720; + + sun6i_csi_bridge_mbus_format_prepare(mbus_format); + + return 0; +} + +static int +sun6i_csi_bridge_enum_mbus_code(struct v4l2_subdev *subdev, + struct v4l2_subdev_state *state, + struct v4l2_subdev_mbus_code_enum *code_enum) +{ + if (code_enum->index >= ARRAY_SIZE(sun6i_csi_bridge_mbus_codes)) + return -EINVAL; + + code_enum->code = sun6i_csi_bridge_mbus_codes[code_enum->index]; + + 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; + + 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; + + 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; + + sun6i_csi_bridge_mbus_format_prepare(mbus_format); + + if (format->which == V4L2_SUBDEV_FORMAT_TRY) + *v4l2_subdev_get_try_format(subdev, state, format->pad) = + *mbus_format; + else + csi_dev->bridge.mbus_format = *mbus_format; + + return 0; +} + +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, + .set_fmt = sun6i_csi_bridge_set_fmt, +}; + +const struct v4l2_subdev_ops sun6i_csi_bridge_subdev_ops = { + .video = &sun6i_csi_bridge_video_ops, + .pad = &sun6i_csi_bridge_pad_ops, +}; + +/* Media Entity */ + +static int sun6i_csi_bridge_link_validate(struct media_link *link) +{ + struct v4l2_subdev *subdev = + media_entity_to_v4l2_subdev(link->sink->entity); + struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev); + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; + struct device *dev = csi_dev->dev; + struct v4l2_subdev *source_subdev = + media_entity_to_v4l2_subdev(link->source->entity); + int ret; + + /* Only support one enabled source at a time. */ + if (bridge->source) { + dev_err(dev, "multiple sources are connected to the bridge\n"); + return -EBUSY; + } + + ret = v4l2_subdev_link_validate(link); + if (ret) + return ret; + + if (source_subdev == bridge->source_parallel.subdev) + bridge->source = &bridge->source_parallel; + else + return -EINVAL; + + return 0; +} + +static const struct media_entity_operations sun6i_csi_bridge_entity_ops = { + .link_validate = sun6i_csi_bridge_link_validate, +}; + +/* V4L2 Async */ + +static int sun6i_csi_bridge_link(struct sun6i_csi_device *csi_dev, + int sink_pad_index, + struct v4l2_subdev *remote_subdev, + bool enabled) +{ + struct device *dev = csi_dev->dev; + struct v4l2_subdev *subdev = &csi_dev->bridge.subdev; + struct media_entity *sink_entity = &subdev->entity; + struct media_entity *source_entity = &remote_subdev->entity; + int source_pad_index; + int ret; + + /* Get the first remote source pad. */ + ret = media_entity_get_fwnode_pad(source_entity, remote_subdev->fwnode, + MEDIA_PAD_FL_SOURCE); + if (ret < 0) { + dev_err(dev, "missing source pad in external entity %s\n", + source_entity->name); + return -EINVAL; + } + + source_pad_index = ret; + + dev_dbg(dev, "creating %s:%u -> %s:%u link\n", source_entity->name, + source_pad_index, sink_entity->name, sink_pad_index); + + ret = media_create_pad_link(source_entity, source_pad_index, + sink_entity, sink_pad_index, + enabled ? MEDIA_LNK_FL_ENABLED : 0); + if (ret < 0) { + dev_err(dev, "failed to create %s:%u -> %s:%u link\n", + source_entity->name, source_pad_index, + sink_entity->name, sink_pad_index); + return ret; + } + + return 0; +} + +static int +sun6i_csi_bridge_notifier_bound(struct v4l2_async_notifier *notifier, + struct v4l2_subdev *remote_subdev, + struct v4l2_async_subdev *async_subdev) +{ + struct sun6i_csi_device *csi_dev = + container_of(notifier, struct sun6i_csi_device, + bridge.notifier); + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; + struct sun6i_csi_bridge_source *source = NULL; + struct fwnode_handle *fwnode = dev_fwnode(csi_dev->dev); + struct fwnode_handle *handle = NULL; + bool enabled; + int ret; + + while ((handle = fwnode_graph_get_next_endpoint(fwnode, handle))) { + struct fwnode_endpoint endpoint = { 0 }; + struct fwnode_handle *remote_fwnode; + + remote_fwnode = fwnode_graph_get_remote_port_parent(handle); + if (!remote_fwnode) + continue; + + if (remote_fwnode != remote_subdev->fwnode) + goto next; + + ret = fwnode_graph_parse_endpoint(handle, &endpoint); + if (ret < 0) + goto next; + + switch (endpoint.port) { + case SUN6I_CSI_PORT_PARALLEL: + source = &bridge->source_parallel; + enabled = true; + break; + default: + break; + } + +next: + fwnode_handle_put(remote_fwnode); + } + + if (!source) + return -EINVAL; + + source->subdev = remote_subdev; + + return sun6i_csi_bridge_link(csi_dev, SUN6I_CSI_BRIDGE_PAD_SINK, + remote_subdev, enabled); +} + +static int +sun6i_csi_bridge_notifier_complete(struct v4l2_async_notifier *notifier) +{ + struct sun6i_csi_device *csi_dev = + container_of(notifier, struct sun6i_csi_device, + bridge.notifier); + + return sun6i_csi_v4l2_complete(csi_dev); +} + +static const struct v4l2_async_notifier_operations +sun6i_csi_bridge_notifier_ops = { + .bound = sun6i_csi_bridge_notifier_bound, + .complete = sun6i_csi_bridge_notifier_complete, +}; + +/* Bridge */ + +static int sun6i_csi_bridge_source_setup(struct sun6i_csi_device *csi_dev, + struct sun6i_csi_bridge_source *source, + u32 port, + enum v4l2_mbus_type *bus_types) +{ + struct device *dev = csi_dev->dev; + struct v4l2_async_notifier *notifier = &csi_dev->bridge.notifier; + struct v4l2_fwnode_endpoint *endpoint = &source->endpoint; + struct v4l2_async_subdev *async_subdev; + struct fwnode_handle *handle; + int ret; + + handle = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), port, 0, 0); + if (!handle) + return -ENODEV; + + ret = v4l2_fwnode_endpoint_parse(handle, endpoint); + if (ret) + goto complete; + + if (bus_types) { + bool valid = false; + unsigned int i; + + for (i = 0; bus_types[i] != V4L2_MBUS_INVALID; i++) { + if (endpoint->bus_type == bus_types[i]) { + valid = true; + break; + } + } + + if (!valid) { + dev_err(dev, "unsupported bus type for port %d\n", + port); + ret = -EINVAL; + goto complete; + } + } + + async_subdev = v4l2_async_nf_add_fwnode_remote(notifier, handle, + struct v4l2_async_subdev); + if (IS_ERR(async_subdev)) { + ret = PTR_ERR(async_subdev); + goto complete; + } + + source->expected = true; + +complete: + fwnode_handle_put(handle); + + return ret; +} + +int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev) +{ + struct device *dev = csi_dev->dev; + struct sun6i_csi_bridge *bridge = &csi_dev->bridge; + struct v4l2_device *v4l2_dev = &csi_dev->v4l2.v4l2_dev; + struct v4l2_subdev *subdev = &bridge->subdev; + struct v4l2_async_notifier *notifier = &bridge->notifier; + struct media_pad *pads = bridge->pads; + enum v4l2_mbus_type parallel_mbus_types[] = { + V4L2_MBUS_PARALLEL, + V4L2_MBUS_BT656, + V4L2_MBUS_INVALID + }; + int ret; + + /* V4L2 Subdev */ + + v4l2_subdev_init(subdev, &sun6i_csi_bridge_subdev_ops); + strscpy(subdev->name, SUN6I_CSI_BRIDGE_NAME, sizeof(subdev->name)); + subdev->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + subdev->owner = THIS_MODULE; + subdev->dev = dev; + + v4l2_set_subdevdata(subdev, csi_dev); + + /* Media Entity */ + + subdev->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; + subdev->entity.ops = &sun6i_csi_bridge_entity_ops; + + /* Media Pads */ + + pads[SUN6I_CSI_BRIDGE_PAD_SINK].flags = MEDIA_PAD_FL_SINK; + pads[SUN6I_CSI_BRIDGE_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE | + MEDIA_PAD_FL_MUST_CONNECT; + + ret = media_entity_pads_init(&subdev->entity, + SUN6I_CSI_BRIDGE_PAD_COUNT, pads); + if (ret < 0) + return ret; + + /* V4L2 Subdev */ + + ret = v4l2_device_register_subdev(v4l2_dev, subdev); + if (ret) { + dev_err(dev, "failed to register v4l2 subdev: %d\n", ret); + goto error_media_entity; + } + + /* V4L2 Async */ + + v4l2_async_nf_init(notifier); + notifier->ops = &sun6i_csi_bridge_notifier_ops; + + sun6i_csi_bridge_source_setup(csi_dev, &bridge->source_parallel, + SUN6I_CSI_PORT_PARALLEL, + parallel_mbus_types); + + ret = v4l2_async_nf_register(v4l2_dev, notifier); + if (ret) { + dev_err(dev, "failed to register v4l2 async notifier: %d\n", + ret); + goto error_v4l2_async_notifier; + } + + return 0; + +error_v4l2_async_notifier: + v4l2_async_nf_cleanup(notifier); + + v4l2_device_unregister_subdev(subdev); + +error_media_entity: + media_entity_cleanup(&subdev->entity); + + return ret; +} + +void sun6i_csi_bridge_cleanup(struct sun6i_csi_device *csi_dev) +{ + struct v4l2_subdev *subdev = &csi_dev->bridge.subdev; + struct v4l2_async_notifier *notifier = &csi_dev->bridge.notifier; + + v4l2_async_nf_unregister(notifier); + v4l2_async_nf_cleanup(notifier); + + v4l2_device_unregister_subdev(subdev); + + media_entity_cleanup(&subdev->entity); +} diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h new file mode 100644 index 000000000000..2ee7878102b6 --- /dev/null +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h @@ -0,0 +1,44 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright 2021 Bootlin + * Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com> + */ + +#ifndef _SUN6I_CSI_BRIDGE_H_ +#define _SUN6I_CSI_BRIDGE_H_ + +#include <media/v4l2-device.h> +#include <media/v4l2-fwnode.h> + +#define SUN6I_CSI_BRIDGE_NAME "sun6i-csi-bridge" + +enum sun6i_csi_bridge_pad { + SUN6I_CSI_BRIDGE_PAD_SINK = 0, + SUN6I_CSI_BRIDGE_PAD_SOURCE = 1, + SUN6I_CSI_BRIDGE_PAD_COUNT = 2, +}; + +struct sun6i_csi_device; + +struct sun6i_csi_bridge_source { + struct v4l2_subdev *subdev; + struct v4l2_fwnode_endpoint endpoint; + bool expected; +}; + +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 sun6i_csi_bridge_source source_parallel; + struct sun6i_csi_bridge_source *source; +}; + +/* Bridge */ + +int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev); +void sun6i_csi_bridge_cleanup(struct sun6i_csi_device *csi_dev); + +#endif diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c index d32ff6b81f8a..fa5bf3697ace 100644 --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c @@ -632,6 +632,7 @@ int sun6i_video_setup(struct sun6i_csi_device *csi_dev) { struct sun6i_video *video = &csi_dev->video; struct v4l2_device *v4l2_dev = &csi_dev->v4l2.v4l2_dev; + struct v4l2_subdev *bridge_subdev = &csi_dev->bridge.subdev; struct video_device *video_dev = &video->video_dev; struct vb2_queue *queue = &video->queue; struct media_pad *pad = &video->pad; @@ -715,8 +716,26 @@ int sun6i_video_setup(struct sun6i_csi_device *csi_dev) v4l2_info(v4l2_dev, "device %s registered as %s\n", video_dev->name, video_device_node_name(video_dev)); + /* Media Pad Link */ + + ret = media_create_pad_link(&bridge_subdev->entity, + SUN6I_CSI_BRIDGE_PAD_SOURCE, + &video_dev->entity, 0, + MEDIA_LNK_FL_ENABLED | + MEDIA_LNK_FL_IMMUTABLE); + if (ret < 0) { + v4l2_err(v4l2_dev, "failed to create %s:%u -> %s:%u link\n", + bridge_subdev->entity.name, + SUN6I_CSI_BRIDGE_PAD_SOURCE, + video_dev->entity.name, 0); + goto error_video_device; + } + return 0; +error_video_device: + vb2_video_unregister_device(video_dev); + error_media_entity: media_entity_cleanup(&video_dev->entity);
Introduce a bridge v4l2 subdev to prepare for separation between the processing part (bridge) and the dma engine, which is required to properly support ths isp workflow later on. Currently the bridge just manages fwnode mapping to media pads, using an async notifier (which was previously in the main code). The s_stream video op just forwards to the connected v4l2 subdev (sensor or MIPI CSI-2 bridge). The video capture device is now registered after the bridge and attaches to it with a media link. Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> --- .../media/platform/sunxi/sun6i-csi/Makefile | 2 +- .../platform/sunxi/sun6i-csi/sun6i_csi.c | 156 +----- .../platform/sunxi/sun6i-csi/sun6i_csi.h | 12 +- .../sunxi/sun6i-csi/sun6i_csi_bridge.c | 473 ++++++++++++++++++ .../sunxi/sun6i-csi/sun6i_csi_bridge.h | 44 ++ .../platform/sunxi/sun6i-csi/sun6i_video.c | 19 + 6 files changed, 571 insertions(+), 135 deletions(-) create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h