| Message ID | 1562082779-31165-4-git-send-email-hugues.fruchet@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DCMI bridge support | expand |
On 7/2/19 5:52 PM, Hugues Fruchet wrote: > Add support of several sub-devices within pipeline instead > of a single one. > This allows to support a CSI-2 camera sensor connected > through a CSI-2 to parallel bridge. > > Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> > --- > drivers/media/platform/stm32/stm32-dcmi.c | 204 +++++++++++++++++++++++++++--- > 1 file changed, 186 insertions(+), 18 deletions(-) > > diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c > index 6f37617..6921e6b 100644 > --- a/drivers/media/platform/stm32/stm32-dcmi.c > +++ b/drivers/media/platform/stm32/stm32-dcmi.c > @@ -172,6 +172,7 @@ struct stm32_dcmi { > > struct media_device mdev; > struct media_pad vid_cap_pad; > + struct media_pipeline pipeline; > }; > > static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n) > @@ -583,6 +584,131 @@ static void dcmi_buf_queue(struct vb2_buffer *vb) > spin_unlock_irq(&dcmi->irqlock); > } > > +static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi) > +{ > + struct media_entity *entity = &dcmi->vdev->entity; > + struct media_pad *pad; > + > + /* Walk searching for entity having no sink */ > + while (1) { > + pad = &entity->pads[0]; > + if (!(pad->flags & MEDIA_PAD_FL_SINK)) > + break; > + > + pad = media_entity_remote_pad(pad); > + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) > + break; > + > + entity = pad->entity; > + } > + > + return entity; > +} > + > +static int dcmi_pipeline_s_fmt(struct stm32_dcmi *dcmi, > + struct v4l2_subdev_pad_config *pad_cfg, > + struct v4l2_subdev_format *format) > +{ > + struct media_entity *entity = &dcmi->entity.source->entity; > + struct v4l2_subdev *subdev; > + struct media_pad *sink_pad = NULL; > + struct media_pad *src_pad = NULL; > + struct media_pad *pad = NULL; > + struct v4l2_subdev_format fmt = *format; > + bool found = false; > + int ret; > + > + /* > + * Starting from sensor subdevice, walk within > + * pipeline and set format on each subdevice > + */ > + while (1) { > + unsigned int i; > + > + /* Search if current entity has a source pad */ > + for (i = 0; i < entity->num_pads; i++) { > + pad = &entity->pads[i]; > + if (pad->flags & MEDIA_PAD_FL_SOURCE) { > + src_pad = pad; > + found = true; > + break; > + } > + } > + if (!found) > + break; > + > + subdev = media_entity_to_v4l2_subdev(entity); > + > + /* Propagate format on sink pad if any, otherwise source pad */ > + if (sink_pad) > + pad = sink_pad; > + > + dev_dbg(dcmi->dev, "%s[%d] pad format set to 0x%x %ux%u\n", > + subdev->name, pad->index, format->format.code, > + format->format.width, format->format.height); > + > + fmt.pad = pad->index; > + ret = v4l2_subdev_call(subdev, pad, set_fmt, pad_cfg, &fmt); > + if (ret < 0) > + return ret; > + > + /* Walk to next entity */ > + sink_pad = media_entity_remote_pad(src_pad); > + if (!sink_pad || !is_media_entity_v4l2_subdev(sink_pad->entity)) > + break; > + > + entity = sink_pad->entity; > + } > + *format = fmt; > + > + return 0; > +} > + > +static int dcmi_pipeline_s_stream(struct stm32_dcmi *dcmi, int state) > +{ > + struct media_entity *entity = &dcmi->vdev->entity; > + struct v4l2_subdev *subdev; > + struct media_pad *pad; > + int ret; > + > + /* Start/stop all entities within pipeline */ > + while (1) { > + pad = &entity->pads[0]; > + if (!(pad->flags & MEDIA_PAD_FL_SINK)) > + break; > + > + pad = media_entity_remote_pad(pad); > + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) > + break; > + > + entity = pad->entity; > + subdev = media_entity_to_v4l2_subdev(entity); > + > + ret = v4l2_subdev_call(subdev, video, s_stream, state); > + if (ret < 0 && ret != -ENOIOCTLCMD) { > + dev_err(dcmi->dev, "%s: %s failed to %s streaming (%d)\n", > + __func__, subdev->name, > + state ? "start" : "stop", ret); > + return ret; > + } > + > + dev_dbg(dcmi->dev, "%s is %s\n", > + subdev->name, state ? "started" : "stopped"); > + } > + > + return 0; > +} > + > +static int dcmi_pipeline_start(struct stm32_dcmi *dcmi) > +{ > + return dcmi_pipeline_s_stream(dcmi, 1); > +} > + > +static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi) > +{ > + dcmi_pipeline_s_stream(dcmi, 0); > +} > + > static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) > { > struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); > @@ -597,14 +723,17 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) > goto err_release_buffers; > } > > - /* Enable stream on the sub device */ > - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 1); > - if (ret && ret != -ENOIOCTLCMD) { > - dev_err(dcmi->dev, "%s: Failed to start streaming, subdev streamon error", > - __func__); > + ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline); > + if (ret < 0) { > + dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n", > + __func__, ret); > goto err_pm_put; > } > > + ret = dcmi_pipeline_start(dcmi); > + if (ret) > + goto err_media_pipeline_stop; > + > spin_lock_irq(&dcmi->irqlock); > > /* Set bus width */ > @@ -676,7 +805,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) > if (ret) { > dev_err(dcmi->dev, "%s: Start streaming failed, cannot start capture\n", > __func__); > - goto err_subdev_streamoff; > + goto err_pipeline_stop; > } > > /* Enable interruptions */ > @@ -687,8 +816,11 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) > > return 0; > > -err_subdev_streamoff: > - v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); > +err_pipeline_stop: > + dcmi_pipeline_stop(dcmi); > + > +err_media_pipeline_stop: > + media_pipeline_stop(&dcmi->vdev->entity); > > err_pm_put: > pm_runtime_put(dcmi->dev); > @@ -713,13 +845,10 @@ static void dcmi_stop_streaming(struct vb2_queue *vq) > { > struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); > struct dcmi_buf *buf, *node; > - int ret; > > - /* Disable stream on the sub device */ > - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); > - if (ret && ret != -ENOIOCTLCMD) > - dev_err(dcmi->dev, "%s: Failed to stop streaming, subdev streamoff error (%d)\n", > - __func__, ret); > + dcmi_pipeline_stop(dcmi); > + > + media_pipeline_stop(&dcmi->vdev->entity); > > spin_lock_irq(&dcmi->irqlock); > > @@ -937,8 +1066,7 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f) > mf->width = sd_framesize.width; > mf->height = sd_framesize.height; > > - ret = v4l2_subdev_call(dcmi->entity.source, pad, > - set_fmt, NULL, &format); > + ret = dcmi_pipeline_s_fmt(dcmi, NULL, &format); > if (ret < 0) > return ret; > > @@ -1529,7 +1657,20 @@ static int dcmi_graph_notify_complete(struct v4l2_async_notifier *notifier) > struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); > int ret; > > + /* > + * Now that the graph is complete, > + * we search for the source subdevice > + * in order to expose it through V4L2 interface > + */ > + dcmi->entity.source = > + media_entity_to_v4l2_subdev(dcmi_find_source(dcmi)); > + if (!dcmi->entity.source) { > + dev_err(dcmi->dev, "Source subdevice not found\n"); > + return -ENODEV; > + } > + > dcmi->vdev->ctrl_handler = dcmi->entity.source->ctrl_handler; > + > ret = dcmi_formats_init(dcmi); > if (ret) { > dev_err(dcmi->dev, "No supported mediabus format found\n"); > @@ -1574,12 +1715,30 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier, > struct v4l2_async_subdev *asd) > { > struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); > + unsigned int ret; > + int src_pad; > > dev_dbg(dcmi->dev, "Subdev %s bound\n", subdev->name); > > - dcmi->entity.source = subdev; > + /* > + * Link this sub-device to DCMI, it could be > + * a parallel camera sensor or a bridge > + */ > + src_pad = media_entity_get_fwnode_pad(&subdev->entity, > + subdev->fwnode, > + MEDIA_PAD_FL_SOURCE); > + > + ret = media_create_pad_link(&subdev->entity, src_pad, > + &dcmi->vdev->entity, 0, > + MEDIA_LNK_FL_IMMUTABLE | > + MEDIA_LNK_FL_ENABLED); > + if (ret) > + dev_err(dcmi->dev, "Failed to create media pad link with subdev %s\n", > + subdev->name); > + else > + dev_dbg(dcmi->dev, "DCMI is now linked to %s\n", subdev->name); > > - return 0; > + return ret; > } > > static const struct v4l2_async_notifier_operations dcmi_graph_notify_ops = { > @@ -1639,6 +1798,15 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi) > return ret; > } > > + /* Register all the subdev nodes */ > + ret = v4l2_device_register_subdev_nodes(&dcmi->v4l2_dev); This shouldn't be needed. Only MC-centric drivers (i.e. where the pipeline has to be configured by userspace) need to do this. Otherwise this patch looks good. Regards, Hans > + if (ret) { > + dev_err(dcmi->dev, "Failed to register subdev nodes\n"); > + v4l2_async_notifier_unregister(&dcmi->notifier); > + of_node_put(dcmi->entity.remote_node); > + return ret; > + } > + > return 0; > } > >
Le jeu. 25 juil. 2019 à 13:40, Hans Verkuil <hverkuil@xs4all.nl> a écrit : > > On 7/2/19 5:52 PM, Hugues Fruchet wrote: > > Add support of several sub-devices within pipeline instead > > of a single one. > > This allows to support a CSI-2 camera sensor connected > > through a CSI-2 to parallel bridge. > > > > Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> > > --- > > drivers/media/platform/stm32/stm32-dcmi.c | 204 +++++++++++++++++++++++++++--- > > 1 file changed, 186 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c > > index 6f37617..6921e6b 100644 > > --- a/drivers/media/platform/stm32/stm32-dcmi.c > > +++ b/drivers/media/platform/stm32/stm32-dcmi.c > > @@ -172,6 +172,7 @@ struct stm32_dcmi { > > > > struct media_device mdev; > > struct media_pad vid_cap_pad; > > + struct media_pipeline pipeline; > > }; > > > > static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n) > > @@ -583,6 +584,131 @@ static void dcmi_buf_queue(struct vb2_buffer *vb) > > spin_unlock_irq(&dcmi->irqlock); > > } > > > > +static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi) > > +{ > > + struct media_entity *entity = &dcmi->vdev->entity; > > + struct media_pad *pad; > > + > > + /* Walk searching for entity having no sink */ > > + while (1) { > > + pad = &entity->pads[0]; > > + if (!(pad->flags & MEDIA_PAD_FL_SINK)) > > + break; > > + > > + pad = media_entity_remote_pad(pad); > > + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) > > + break; > > + > > + entity = pad->entity; > > + } > > + > > + return entity; > > +} > > + > > +static int dcmi_pipeline_s_fmt(struct stm32_dcmi *dcmi, > > + struct v4l2_subdev_pad_config *pad_cfg, > > + struct v4l2_subdev_format *format) > > +{ > > + struct media_entity *entity = &dcmi->entity.source->entity; > > + struct v4l2_subdev *subdev; > > + struct media_pad *sink_pad = NULL; > > + struct media_pad *src_pad = NULL; > > + struct media_pad *pad = NULL; > > + struct v4l2_subdev_format fmt = *format; > > + bool found = false; > > + int ret; > > + > > + /* > > + * Starting from sensor subdevice, walk within > > + * pipeline and set format on each subdevice > > + */ > > + while (1) { > > + unsigned int i; > > + > > + /* Search if current entity has a source pad */ > > + for (i = 0; i < entity->num_pads; i++) { > > + pad = &entity->pads[i]; > > + if (pad->flags & MEDIA_PAD_FL_SOURCE) { > > + src_pad = pad; > > + found = true; > > + break; > > + } > > + } > > + if (!found) > > + break; > > + > > + subdev = media_entity_to_v4l2_subdev(entity); > > + > > + /* Propagate format on sink pad if any, otherwise source pad */ > > + if (sink_pad) > > + pad = sink_pad; > > + > > + dev_dbg(dcmi->dev, "%s[%d] pad format set to 0x%x %ux%u\n", > > + subdev->name, pad->index, format->format.code, > > + format->format.width, format->format.height); > > + > > + fmt.pad = pad->index; > > + ret = v4l2_subdev_call(subdev, pad, set_fmt, pad_cfg, &fmt); > > + if (ret < 0) > > + return ret; > > + > > + /* Walk to next entity */ > > + sink_pad = media_entity_remote_pad(src_pad); > > + if (!sink_pad || !is_media_entity_v4l2_subdev(sink_pad->entity)) > > + break; > > + > > + entity = sink_pad->entity; > > + } > > + *format = fmt; > > + > > + return 0; > > +} > > + > > +static int dcmi_pipeline_s_stream(struct stm32_dcmi *dcmi, int state) > > +{ > > + struct media_entity *entity = &dcmi->vdev->entity; > > + struct v4l2_subdev *subdev; > > + struct media_pad *pad; > > + int ret; > > + > > + /* Start/stop all entities within pipeline */ > > + while (1) { > > + pad = &entity->pads[0]; > > + if (!(pad->flags & MEDIA_PAD_FL_SINK)) > > + break; > > + > > + pad = media_entity_remote_pad(pad); > > + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) > > + break; > > + > > + entity = pad->entity; > > + subdev = media_entity_to_v4l2_subdev(entity); > > + > > + ret = v4l2_subdev_call(subdev, video, s_stream, state); > > + if (ret < 0 && ret != -ENOIOCTLCMD) { > > + dev_err(dcmi->dev, "%s: %s failed to %s streaming (%d)\n", > > + __func__, subdev->name, > > + state ? "start" : "stop", ret); > > + return ret; > > + } > > + > > + dev_dbg(dcmi->dev, "%s is %s\n", > > + subdev->name, state ? "started" : "stopped"); > > + } > > + > > + return 0; > > +} > > + > > +static int dcmi_pipeline_start(struct stm32_dcmi *dcmi) > > +{ > > + return dcmi_pipeline_s_stream(dcmi, 1); > > +} > > + > > +static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi) > > +{ > > + dcmi_pipeline_s_stream(dcmi, 0); > > +} > > + > > static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) > > { > > struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); > > @@ -597,14 +723,17 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) > > goto err_release_buffers; > > } > > > > - /* Enable stream on the sub device */ > > - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 1); > > - if (ret && ret != -ENOIOCTLCMD) { > > - dev_err(dcmi->dev, "%s: Failed to start streaming, subdev streamon error", > > - __func__); > > + ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline); > > + if (ret < 0) { > > + dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n", > > + __func__, ret); > > goto err_pm_put; > > } > > > > + ret = dcmi_pipeline_start(dcmi); > > + if (ret) > > + goto err_media_pipeline_stop; > > + > > spin_lock_irq(&dcmi->irqlock); > > > > /* Set bus width */ > > @@ -676,7 +805,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) > > if (ret) { > > dev_err(dcmi->dev, "%s: Start streaming failed, cannot start capture\n", > > __func__); > > - goto err_subdev_streamoff; > > + goto err_pipeline_stop; > > } > > > > /* Enable interruptions */ > > @@ -687,8 +816,11 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) > > > > return 0; > > > > -err_subdev_streamoff: > > - v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); > > +err_pipeline_stop: > > + dcmi_pipeline_stop(dcmi); > > + > > +err_media_pipeline_stop: > > + media_pipeline_stop(&dcmi->vdev->entity); > > > > err_pm_put: > > pm_runtime_put(dcmi->dev); > > @@ -713,13 +845,10 @@ static void dcmi_stop_streaming(struct vb2_queue *vq) > > { > > struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); > > struct dcmi_buf *buf, *node; > > - int ret; > > > > - /* Disable stream on the sub device */ > > - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); > > - if (ret && ret != -ENOIOCTLCMD) > > - dev_err(dcmi->dev, "%s: Failed to stop streaming, subdev streamoff error (%d)\n", > > - __func__, ret); > > + dcmi_pipeline_stop(dcmi); > > + > > + media_pipeline_stop(&dcmi->vdev->entity); > > > > spin_lock_irq(&dcmi->irqlock); > > > > @@ -937,8 +1066,7 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f) > > mf->width = sd_framesize.width; > > mf->height = sd_framesize.height; > > > > - ret = v4l2_subdev_call(dcmi->entity.source, pad, > > - set_fmt, NULL, &format); > > + ret = dcmi_pipeline_s_fmt(dcmi, NULL, &format); > > if (ret < 0) > > return ret; > > > > @@ -1529,7 +1657,20 @@ static int dcmi_graph_notify_complete(struct v4l2_async_notifier *notifier) > > struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); > > int ret; > > > > + /* > > + * Now that the graph is complete, > > + * we search for the source subdevice > > + * in order to expose it through V4L2 interface > > + */ > > + dcmi->entity.source = > > + media_entity_to_v4l2_subdev(dcmi_find_source(dcmi)); > > + if (!dcmi->entity.source) { > > + dev_err(dcmi->dev, "Source subdevice not found\n"); > > + return -ENODEV; > > + } > > + > > dcmi->vdev->ctrl_handler = dcmi->entity.source->ctrl_handler; > > + > > ret = dcmi_formats_init(dcmi); > > if (ret) { > > dev_err(dcmi->dev, "No supported mediabus format found\n"); > > @@ -1574,12 +1715,30 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier, > > struct v4l2_async_subdev *asd) > > { > > struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); > > + unsigned int ret; > > + int src_pad; > > > > dev_dbg(dcmi->dev, "Subdev %s bound\n", subdev->name); > > > > - dcmi->entity.source = subdev; > > + /* > > + * Link this sub-device to DCMI, it could be > > + * a parallel camera sensor or a bridge > > + */ > > + src_pad = media_entity_get_fwnode_pad(&subdev->entity, > > + subdev->fwnode, > > + MEDIA_PAD_FL_SOURCE); > > + > > + ret = media_create_pad_link(&subdev->entity, src_pad, > > + &dcmi->vdev->entity, 0, > > + MEDIA_LNK_FL_IMMUTABLE | > > + MEDIA_LNK_FL_ENABLED); > > + if (ret) > > + dev_err(dcmi->dev, "Failed to create media pad link with subdev %s\n", > > + subdev->name); > > + else > > + dev_dbg(dcmi->dev, "DCMI is now linked to %s\n", subdev->name); > > > > - return 0; > > + return ret; > > } > > > > static const struct v4l2_async_notifier_operations dcmi_graph_notify_ops = { > > @@ -1639,6 +1798,15 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi) > > return ret; > > } > > > > + /* Register all the subdev nodes */ > > + ret = v4l2_device_register_subdev_nodes(&dcmi->v4l2_dev); > > This shouldn't be needed. Only MC-centric drivers (i.e. where the pipeline > has to be configured by userspace) need to do this. Hi Hans, I think this point has been discussed in this thread https://www.spinics.net/lists/linux-media/msg153417.html In short : since the hardware only offer one possible path we don't expose the configuration to userland and let DCMI driver configure the subdevice (like bridge). Benjamin > > Otherwise this patch looks good. > > Regards, > > Hans > > > + if (ret) { > > + dev_err(dcmi->dev, "Failed to register subdev nodes\n"); > > + v4l2_async_notifier_unregister(&dcmi->notifier); > > + of_node_put(dcmi->entity.remote_node); > > + return ret; > > + } > > + > > return 0; > > } > > > > >
On 7/25/19 4:56 PM, Benjamin Gaignard wrote: > Le jeu. 25 juil. 2019 à 13:40, Hans Verkuil <hverkuil@xs4all.nl> a écrit : >> >> On 7/2/19 5:52 PM, Hugues Fruchet wrote: >>> Add support of several sub-devices within pipeline instead >>> of a single one. >>> This allows to support a CSI-2 camera sensor connected >>> through a CSI-2 to parallel bridge. >>> >>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> >>> --- >>> drivers/media/platform/stm32/stm32-dcmi.c | 204 +++++++++++++++++++++++++++--- >>> 1 file changed, 186 insertions(+), 18 deletions(-) >>> >>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c >>> index 6f37617..6921e6b 100644 >>> --- a/drivers/media/platform/stm32/stm32-dcmi.c >>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c >>> @@ -172,6 +172,7 @@ struct stm32_dcmi { >>> >>> struct media_device mdev; >>> struct media_pad vid_cap_pad; >>> + struct media_pipeline pipeline; >>> }; >>> >>> static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n) >>> @@ -583,6 +584,131 @@ static void dcmi_buf_queue(struct vb2_buffer *vb) >>> spin_unlock_irq(&dcmi->irqlock); >>> } >>> >>> +static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi) >>> +{ >>> + struct media_entity *entity = &dcmi->vdev->entity; >>> + struct media_pad *pad; >>> + >>> + /* Walk searching for entity having no sink */ >>> + while (1) { >>> + pad = &entity->pads[0]; >>> + if (!(pad->flags & MEDIA_PAD_FL_SINK)) >>> + break; >>> + >>> + pad = media_entity_remote_pad(pad); >>> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) >>> + break; >>> + >>> + entity = pad->entity; >>> + } >>> + >>> + return entity; >>> +} >>> + >>> +static int dcmi_pipeline_s_fmt(struct stm32_dcmi *dcmi, >>> + struct v4l2_subdev_pad_config *pad_cfg, >>> + struct v4l2_subdev_format *format) >>> +{ >>> + struct media_entity *entity = &dcmi->entity.source->entity; >>> + struct v4l2_subdev *subdev; >>> + struct media_pad *sink_pad = NULL; >>> + struct media_pad *src_pad = NULL; >>> + struct media_pad *pad = NULL; >>> + struct v4l2_subdev_format fmt = *format; >>> + bool found = false; >>> + int ret; >>> + >>> + /* >>> + * Starting from sensor subdevice, walk within >>> + * pipeline and set format on each subdevice >>> + */ >>> + while (1) { >>> + unsigned int i; >>> + >>> + /* Search if current entity has a source pad */ >>> + for (i = 0; i < entity->num_pads; i++) { >>> + pad = &entity->pads[i]; >>> + if (pad->flags & MEDIA_PAD_FL_SOURCE) { >>> + src_pad = pad; >>> + found = true; >>> + break; >>> + } >>> + } >>> + if (!found) >>> + break; >>> + >>> + subdev = media_entity_to_v4l2_subdev(entity); >>> + >>> + /* Propagate format on sink pad if any, otherwise source pad */ >>> + if (sink_pad) >>> + pad = sink_pad; >>> + >>> + dev_dbg(dcmi->dev, "%s[%d] pad format set to 0x%x %ux%u\n", >>> + subdev->name, pad->index, format->format.code, >>> + format->format.width, format->format.height); >>> + >>> + fmt.pad = pad->index; >>> + ret = v4l2_subdev_call(subdev, pad, set_fmt, pad_cfg, &fmt); >>> + if (ret < 0) >>> + return ret; >>> + >>> + /* Walk to next entity */ >>> + sink_pad = media_entity_remote_pad(src_pad); >>> + if (!sink_pad || !is_media_entity_v4l2_subdev(sink_pad->entity)) >>> + break; >>> + >>> + entity = sink_pad->entity; >>> + } >>> + *format = fmt; >>> + >>> + return 0; >>> +} >>> + >>> +static int dcmi_pipeline_s_stream(struct stm32_dcmi *dcmi, int state) >>> +{ >>> + struct media_entity *entity = &dcmi->vdev->entity; >>> + struct v4l2_subdev *subdev; >>> + struct media_pad *pad; >>> + int ret; >>> + >>> + /* Start/stop all entities within pipeline */ >>> + while (1) { >>> + pad = &entity->pads[0]; >>> + if (!(pad->flags & MEDIA_PAD_FL_SINK)) >>> + break; >>> + >>> + pad = media_entity_remote_pad(pad); >>> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) >>> + break; >>> + >>> + entity = pad->entity; >>> + subdev = media_entity_to_v4l2_subdev(entity); >>> + >>> + ret = v4l2_subdev_call(subdev, video, s_stream, state); >>> + if (ret < 0 && ret != -ENOIOCTLCMD) { >>> + dev_err(dcmi->dev, "%s: %s failed to %s streaming (%d)\n", >>> + __func__, subdev->name, >>> + state ? "start" : "stop", ret); >>> + return ret; >>> + } >>> + >>> + dev_dbg(dcmi->dev, "%s is %s\n", >>> + subdev->name, state ? "started" : "stopped"); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int dcmi_pipeline_start(struct stm32_dcmi *dcmi) >>> +{ >>> + return dcmi_pipeline_s_stream(dcmi, 1); >>> +} >>> + >>> +static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi) >>> +{ >>> + dcmi_pipeline_s_stream(dcmi, 0); >>> +} >>> + >>> static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) >>> { >>> struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); >>> @@ -597,14 +723,17 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) >>> goto err_release_buffers; >>> } >>> >>> - /* Enable stream on the sub device */ >>> - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 1); >>> - if (ret && ret != -ENOIOCTLCMD) { >>> - dev_err(dcmi->dev, "%s: Failed to start streaming, subdev streamon error", >>> - __func__); >>> + ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline); >>> + if (ret < 0) { >>> + dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n", >>> + __func__, ret); >>> goto err_pm_put; >>> } >>> >>> + ret = dcmi_pipeline_start(dcmi); >>> + if (ret) >>> + goto err_media_pipeline_stop; >>> + >>> spin_lock_irq(&dcmi->irqlock); >>> >>> /* Set bus width */ >>> @@ -676,7 +805,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) >>> if (ret) { >>> dev_err(dcmi->dev, "%s: Start streaming failed, cannot start capture\n", >>> __func__); >>> - goto err_subdev_streamoff; >>> + goto err_pipeline_stop; >>> } >>> >>> /* Enable interruptions */ >>> @@ -687,8 +816,11 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) >>> >>> return 0; >>> >>> -err_subdev_streamoff: >>> - v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); >>> +err_pipeline_stop: >>> + dcmi_pipeline_stop(dcmi); >>> + >>> +err_media_pipeline_stop: >>> + media_pipeline_stop(&dcmi->vdev->entity); >>> >>> err_pm_put: >>> pm_runtime_put(dcmi->dev); >>> @@ -713,13 +845,10 @@ static void dcmi_stop_streaming(struct vb2_queue *vq) >>> { >>> struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); >>> struct dcmi_buf *buf, *node; >>> - int ret; >>> >>> - /* Disable stream on the sub device */ >>> - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); >>> - if (ret && ret != -ENOIOCTLCMD) >>> - dev_err(dcmi->dev, "%s: Failed to stop streaming, subdev streamoff error (%d)\n", >>> - __func__, ret); >>> + dcmi_pipeline_stop(dcmi); >>> + >>> + media_pipeline_stop(&dcmi->vdev->entity); >>> >>> spin_lock_irq(&dcmi->irqlock); >>> >>> @@ -937,8 +1066,7 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f) >>> mf->width = sd_framesize.width; >>> mf->height = sd_framesize.height; >>> >>> - ret = v4l2_subdev_call(dcmi->entity.source, pad, >>> - set_fmt, NULL, &format); >>> + ret = dcmi_pipeline_s_fmt(dcmi, NULL, &format); >>> if (ret < 0) >>> return ret; >>> >>> @@ -1529,7 +1657,20 @@ static int dcmi_graph_notify_complete(struct v4l2_async_notifier *notifier) >>> struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); >>> int ret; >>> >>> + /* >>> + * Now that the graph is complete, >>> + * we search for the source subdevice >>> + * in order to expose it through V4L2 interface >>> + */ >>> + dcmi->entity.source = >>> + media_entity_to_v4l2_subdev(dcmi_find_source(dcmi)); >>> + if (!dcmi->entity.source) { >>> + dev_err(dcmi->dev, "Source subdevice not found\n"); >>> + return -ENODEV; >>> + } >>> + >>> dcmi->vdev->ctrl_handler = dcmi->entity.source->ctrl_handler; >>> + >>> ret = dcmi_formats_init(dcmi); >>> if (ret) { >>> dev_err(dcmi->dev, "No supported mediabus format found\n"); >>> @@ -1574,12 +1715,30 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier, >>> struct v4l2_async_subdev *asd) >>> { >>> struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); >>> + unsigned int ret; >>> + int src_pad; >>> >>> dev_dbg(dcmi->dev, "Subdev %s bound\n", subdev->name); >>> >>> - dcmi->entity.source = subdev; >>> + /* >>> + * Link this sub-device to DCMI, it could be >>> + * a parallel camera sensor or a bridge >>> + */ >>> + src_pad = media_entity_get_fwnode_pad(&subdev->entity, >>> + subdev->fwnode, >>> + MEDIA_PAD_FL_SOURCE); >>> + >>> + ret = media_create_pad_link(&subdev->entity, src_pad, >>> + &dcmi->vdev->entity, 0, >>> + MEDIA_LNK_FL_IMMUTABLE | >>> + MEDIA_LNK_FL_ENABLED); >>> + if (ret) >>> + dev_err(dcmi->dev, "Failed to create media pad link with subdev %s\n", >>> + subdev->name); >>> + else >>> + dev_dbg(dcmi->dev, "DCMI is now linked to %s\n", subdev->name); >>> >>> - return 0; >>> + return ret; >>> } >>> >>> static const struct v4l2_async_notifier_operations dcmi_graph_notify_ops = { >>> @@ -1639,6 +1798,15 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi) >>> return ret; >>> } >>> >>> + /* Register all the subdev nodes */ >>> + ret = v4l2_device_register_subdev_nodes(&dcmi->v4l2_dev); >> >> This shouldn't be needed. Only MC-centric drivers (i.e. where the pipeline >> has to be configured by userspace) need to do this. > > Hi Hans, > I think this point has been discussed in this thread > https://www.spinics.net/lists/linux-media/msg153417.html > > In short : since the hardware only offer one possible path we don't expose > the configuration to userland and let DCMI driver configure the > subdevice (like bridge). Yes, and I agree completely. But why then do you register v4l-subdevX devices? That's not needed for this driver. Regards, Hans > > Benjamin > >> >> Otherwise this patch looks good. >> >> Regards, >> >> Hans >> >>> + if (ret) { >>> + dev_err(dcmi->dev, "Failed to register subdev nodes\n"); >>> + v4l2_async_notifier_unregister(&dcmi->notifier); >>> + of_node_put(dcmi->entity.remote_node); >>> + return ret; >>> + } >>> + >>> return 0; >>> } >>> >>> >>
Hi Hans, I've just pushed v4 with subdev nodes registry removal as per your suggestion. On 7/25/19 5:09 PM, Hans Verkuil wrote: > On 7/25/19 4:56 PM, Benjamin Gaignard wrote: >> Le jeu. 25 juil. 2019 à 13:40, Hans Verkuil <hverkuil@xs4all.nl> a écrit : >>> >>> On 7/2/19 5:52 PM, Hugues Fruchet wrote: >>>> Add support of several sub-devices within pipeline instead >>>> of a single one. >>>> This allows to support a CSI-2 camera sensor connected >>>> through a CSI-2 to parallel bridge. >>>> >>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> >>>> --- >>>> drivers/media/platform/stm32/stm32-dcmi.c | 204 +++++++++++++++++++++++++++--- >>>> 1 file changed, 186 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c >>>> index 6f37617..6921e6b 100644 >>>> --- a/drivers/media/platform/stm32/stm32-dcmi.c >>>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c >>>> @@ -172,6 +172,7 @@ struct stm32_dcmi { >>>> >>>> struct media_device mdev; >>>> struct media_pad vid_cap_pad; >>>> + struct media_pipeline pipeline; >>>> }; >>>> >>>> static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n) >>>> @@ -583,6 +584,131 @@ static void dcmi_buf_queue(struct vb2_buffer *vb) >>>> spin_unlock_irq(&dcmi->irqlock); >>>> } >>>> >>>> +static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi) >>>> +{ >>>> + struct media_entity *entity = &dcmi->vdev->entity; >>>> + struct media_pad *pad; >>>> + >>>> + /* Walk searching for entity having no sink */ >>>> + while (1) { >>>> + pad = &entity->pads[0]; >>>> + if (!(pad->flags & MEDIA_PAD_FL_SINK)) >>>> + break; >>>> + >>>> + pad = media_entity_remote_pad(pad); >>>> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) >>>> + break; >>>> + >>>> + entity = pad->entity; >>>> + } >>>> + >>>> + return entity; >>>> +} >>>> + >>>> +static int dcmi_pipeline_s_fmt(struct stm32_dcmi *dcmi, >>>> + struct v4l2_subdev_pad_config *pad_cfg, >>>> + struct v4l2_subdev_format *format) >>>> +{ >>>> + struct media_entity *entity = &dcmi->entity.source->entity; >>>> + struct v4l2_subdev *subdev; >>>> + struct media_pad *sink_pad = NULL; >>>> + struct media_pad *src_pad = NULL; >>>> + struct media_pad *pad = NULL; >>>> + struct v4l2_subdev_format fmt = *format; >>>> + bool found = false; >>>> + int ret; >>>> + >>>> + /* >>>> + * Starting from sensor subdevice, walk within >>>> + * pipeline and set format on each subdevice >>>> + */ >>>> + while (1) { >>>> + unsigned int i; >>>> + >>>> + /* Search if current entity has a source pad */ >>>> + for (i = 0; i < entity->num_pads; i++) { >>>> + pad = &entity->pads[i]; >>>> + if (pad->flags & MEDIA_PAD_FL_SOURCE) { >>>> + src_pad = pad; >>>> + found = true; >>>> + break; >>>> + } >>>> + } >>>> + if (!found) >>>> + break; >>>> + >>>> + subdev = media_entity_to_v4l2_subdev(entity); >>>> + >>>> + /* Propagate format on sink pad if any, otherwise source pad */ >>>> + if (sink_pad) >>>> + pad = sink_pad; >>>> + >>>> + dev_dbg(dcmi->dev, "%s[%d] pad format set to 0x%x %ux%u\n", >>>> + subdev->name, pad->index, format->format.code, >>>> + format->format.width, format->format.height); >>>> + >>>> + fmt.pad = pad->index; >>>> + ret = v4l2_subdev_call(subdev, pad, set_fmt, pad_cfg, &fmt); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> + /* Walk to next entity */ >>>> + sink_pad = media_entity_remote_pad(src_pad); >>>> + if (!sink_pad || !is_media_entity_v4l2_subdev(sink_pad->entity)) >>>> + break; >>>> + >>>> + entity = sink_pad->entity; >>>> + } >>>> + *format = fmt; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int dcmi_pipeline_s_stream(struct stm32_dcmi *dcmi, int state) >>>> +{ >>>> + struct media_entity *entity = &dcmi->vdev->entity; >>>> + struct v4l2_subdev *subdev; >>>> + struct media_pad *pad; >>>> + int ret; >>>> + >>>> + /* Start/stop all entities within pipeline */ >>>> + while (1) { >>>> + pad = &entity->pads[0]; >>>> + if (!(pad->flags & MEDIA_PAD_FL_SINK)) >>>> + break; >>>> + >>>> + pad = media_entity_remote_pad(pad); >>>> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) >>>> + break; >>>> + >>>> + entity = pad->entity; >>>> + subdev = media_entity_to_v4l2_subdev(entity); >>>> + >>>> + ret = v4l2_subdev_call(subdev, video, s_stream, state); >>>> + if (ret < 0 && ret != -ENOIOCTLCMD) { >>>> + dev_err(dcmi->dev, "%s: %s failed to %s streaming (%d)\n", >>>> + __func__, subdev->name, >>>> + state ? "start" : "stop", ret); >>>> + return ret; >>>> + } >>>> + >>>> + dev_dbg(dcmi->dev, "%s is %s\n", >>>> + subdev->name, state ? "started" : "stopped"); >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int dcmi_pipeline_start(struct stm32_dcmi *dcmi) >>>> +{ >>>> + return dcmi_pipeline_s_stream(dcmi, 1); >>>> +} >>>> + >>>> +static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi) >>>> +{ >>>> + dcmi_pipeline_s_stream(dcmi, 0); >>>> +} >>>> + >>>> static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) >>>> { >>>> struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); >>>> @@ -597,14 +723,17 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) >>>> goto err_release_buffers; >>>> } >>>> >>>> - /* Enable stream on the sub device */ >>>> - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 1); >>>> - if (ret && ret != -ENOIOCTLCMD) { >>>> - dev_err(dcmi->dev, "%s: Failed to start streaming, subdev streamon error", >>>> - __func__); >>>> + ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline); >>>> + if (ret < 0) { >>>> + dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n", >>>> + __func__, ret); >>>> goto err_pm_put; >>>> } >>>> >>>> + ret = dcmi_pipeline_start(dcmi); >>>> + if (ret) >>>> + goto err_media_pipeline_stop; >>>> + >>>> spin_lock_irq(&dcmi->irqlock); >>>> >>>> /* Set bus width */ >>>> @@ -676,7 +805,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) >>>> if (ret) { >>>> dev_err(dcmi->dev, "%s: Start streaming failed, cannot start capture\n", >>>> __func__); >>>> - goto err_subdev_streamoff; >>>> + goto err_pipeline_stop; >>>> } >>>> >>>> /* Enable interruptions */ >>>> @@ -687,8 +816,11 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) >>>> >>>> return 0; >>>> >>>> -err_subdev_streamoff: >>>> - v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); >>>> +err_pipeline_stop: >>>> + dcmi_pipeline_stop(dcmi); >>>> + >>>> +err_media_pipeline_stop: >>>> + media_pipeline_stop(&dcmi->vdev->entity); >>>> >>>> err_pm_put: >>>> pm_runtime_put(dcmi->dev); >>>> @@ -713,13 +845,10 @@ static void dcmi_stop_streaming(struct vb2_queue *vq) >>>> { >>>> struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); >>>> struct dcmi_buf *buf, *node; >>>> - int ret; >>>> >>>> - /* Disable stream on the sub device */ >>>> - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); >>>> - if (ret && ret != -ENOIOCTLCMD) >>>> - dev_err(dcmi->dev, "%s: Failed to stop streaming, subdev streamoff error (%d)\n", >>>> - __func__, ret); >>>> + dcmi_pipeline_stop(dcmi); >>>> + >>>> + media_pipeline_stop(&dcmi->vdev->entity); >>>> >>>> spin_lock_irq(&dcmi->irqlock); >>>> >>>> @@ -937,8 +1066,7 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f) >>>> mf->width = sd_framesize.width; >>>> mf->height = sd_framesize.height; >>>> >>>> - ret = v4l2_subdev_call(dcmi->entity.source, pad, >>>> - set_fmt, NULL, &format); >>>> + ret = dcmi_pipeline_s_fmt(dcmi, NULL, &format); >>>> if (ret < 0) >>>> return ret; >>>> >>>> @@ -1529,7 +1657,20 @@ static int dcmi_graph_notify_complete(struct v4l2_async_notifier *notifier) >>>> struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); >>>> int ret; >>>> >>>> + /* >>>> + * Now that the graph is complete, >>>> + * we search for the source subdevice >>>> + * in order to expose it through V4L2 interface >>>> + */ >>>> + dcmi->entity.source = >>>> + media_entity_to_v4l2_subdev(dcmi_find_source(dcmi)); >>>> + if (!dcmi->entity.source) { >>>> + dev_err(dcmi->dev, "Source subdevice not found\n"); >>>> + return -ENODEV; >>>> + } >>>> + >>>> dcmi->vdev->ctrl_handler = dcmi->entity.source->ctrl_handler; >>>> + >>>> ret = dcmi_formats_init(dcmi); >>>> if (ret) { >>>> dev_err(dcmi->dev, "No supported mediabus format found\n"); >>>> @@ -1574,12 +1715,30 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier, >>>> struct v4l2_async_subdev *asd) >>>> { >>>> struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); >>>> + unsigned int ret; >>>> + int src_pad; >>>> >>>> dev_dbg(dcmi->dev, "Subdev %s bound\n", subdev->name); >>>> >>>> - dcmi->entity.source = subdev; >>>> + /* >>>> + * Link this sub-device to DCMI, it could be >>>> + * a parallel camera sensor or a bridge >>>> + */ >>>> + src_pad = media_entity_get_fwnode_pad(&subdev->entity, >>>> + subdev->fwnode, >>>> + MEDIA_PAD_FL_SOURCE); >>>> + >>>> + ret = media_create_pad_link(&subdev->entity, src_pad, >>>> + &dcmi->vdev->entity, 0, >>>> + MEDIA_LNK_FL_IMMUTABLE | >>>> + MEDIA_LNK_FL_ENABLED); >>>> + if (ret) >>>> + dev_err(dcmi->dev, "Failed to create media pad link with subdev %s\n", >>>> + subdev->name); >>>> + else >>>> + dev_dbg(dcmi->dev, "DCMI is now linked to %s\n", subdev->name); >>>> >>>> - return 0; >>>> + return ret; >>>> } >>>> >>>> static const struct v4l2_async_notifier_operations dcmi_graph_notify_ops = { >>>> @@ -1639,6 +1798,15 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi) >>>> return ret; >>>> } >>>> >>>> + /* Register all the subdev nodes */ >>>> + ret = v4l2_device_register_subdev_nodes(&dcmi->v4l2_dev); >>> >>> This shouldn't be needed. Only MC-centric drivers (i.e. where the pipeline >>> has to be configured by userspace) need to do this. >> >> Hi Hans, >> I think this point has been discussed in this thread >> https://www.spinics.net/lists/linux-media/msg153417.html >> >> In short : since the hardware only offer one possible path we don't expose >> the configuration to userland and let DCMI driver configure the >> subdevice (like bridge). > > Yes, and I agree completely. But why then do you register v4l-subdevX devices? > That's not needed for this driver. > > Regards, > > Hans > >> >> Benjamin >> >>> >>> Otherwise this patch looks good. >>> >>> Regards, >>> >>> Hans >>> >>>> + if (ret) { >>>> + dev_err(dcmi->dev, "Failed to register subdev nodes\n"); >>>> + v4l2_async_notifier_unregister(&dcmi->notifier); >>>> + of_node_put(dcmi->entity.remote_node); >>>> + return ret; >>>> + } >>>> + >>>> return 0; >>>> } >>>> >>>> >>> > Best regards, Hugues.
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c index 6f37617..6921e6b 100644 --- a/drivers/media/platform/stm32/stm32-dcmi.c +++ b/drivers/media/platform/stm32/stm32-dcmi.c @@ -172,6 +172,7 @@ struct stm32_dcmi { struct media_device mdev; struct media_pad vid_cap_pad; + struct media_pipeline pipeline; }; static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n) @@ -583,6 +584,131 @@ static void dcmi_buf_queue(struct vb2_buffer *vb) spin_unlock_irq(&dcmi->irqlock); } +static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi) +{ + struct media_entity *entity = &dcmi->vdev->entity; + struct media_pad *pad; + + /* Walk searching for entity having no sink */ + while (1) { + pad = &entity->pads[0]; + if (!(pad->flags & MEDIA_PAD_FL_SINK)) + break; + + pad = media_entity_remote_pad(pad); + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) + break; + + entity = pad->entity; + } + + return entity; +} + +static int dcmi_pipeline_s_fmt(struct stm32_dcmi *dcmi, + struct v4l2_subdev_pad_config *pad_cfg, + struct v4l2_subdev_format *format) +{ + struct media_entity *entity = &dcmi->entity.source->entity; + struct v4l2_subdev *subdev; + struct media_pad *sink_pad = NULL; + struct media_pad *src_pad = NULL; + struct media_pad *pad = NULL; + struct v4l2_subdev_format fmt = *format; + bool found = false; + int ret; + + /* + * Starting from sensor subdevice, walk within + * pipeline and set format on each subdevice + */ + while (1) { + unsigned int i; + + /* Search if current entity has a source pad */ + for (i = 0; i < entity->num_pads; i++) { + pad = &entity->pads[i]; + if (pad->flags & MEDIA_PAD_FL_SOURCE) { + src_pad = pad; + found = true; + break; + } + } + if (!found) + break; + + subdev = media_entity_to_v4l2_subdev(entity); + + /* Propagate format on sink pad if any, otherwise source pad */ + if (sink_pad) + pad = sink_pad; + + dev_dbg(dcmi->dev, "%s[%d] pad format set to 0x%x %ux%u\n", + subdev->name, pad->index, format->format.code, + format->format.width, format->format.height); + + fmt.pad = pad->index; + ret = v4l2_subdev_call(subdev, pad, set_fmt, pad_cfg, &fmt); + if (ret < 0) + return ret; + + /* Walk to next entity */ + sink_pad = media_entity_remote_pad(src_pad); + if (!sink_pad || !is_media_entity_v4l2_subdev(sink_pad->entity)) + break; + + entity = sink_pad->entity; + } + *format = fmt; + + return 0; +} + +static int dcmi_pipeline_s_stream(struct stm32_dcmi *dcmi, int state) +{ + struct media_entity *entity = &dcmi->vdev->entity; + struct v4l2_subdev *subdev; + struct media_pad *pad; + int ret; + + /* Start/stop all entities within pipeline */ + while (1) { + pad = &entity->pads[0]; + if (!(pad->flags & MEDIA_PAD_FL_SINK)) + break; + + pad = media_entity_remote_pad(pad); + if (!pad || !is_media_entity_v4l2_subdev(pad->entity)) + break; + + entity = pad->entity; + subdev = media_entity_to_v4l2_subdev(entity); + + ret = v4l2_subdev_call(subdev, video, s_stream, state); + if (ret < 0 && ret != -ENOIOCTLCMD) { + dev_err(dcmi->dev, "%s: %s failed to %s streaming (%d)\n", + __func__, subdev->name, + state ? "start" : "stop", ret); + return ret; + } + + dev_dbg(dcmi->dev, "%s is %s\n", + subdev->name, state ? "started" : "stopped"); + } + + return 0; +} + +static int dcmi_pipeline_start(struct stm32_dcmi *dcmi) +{ + return dcmi_pipeline_s_stream(dcmi, 1); +} + +static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi) +{ + dcmi_pipeline_s_stream(dcmi, 0); +} + static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) { struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); @@ -597,14 +723,17 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) goto err_release_buffers; } - /* Enable stream on the sub device */ - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 1); - if (ret && ret != -ENOIOCTLCMD) { - dev_err(dcmi->dev, "%s: Failed to start streaming, subdev streamon error", - __func__); + ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline); + if (ret < 0) { + dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n", + __func__, ret); goto err_pm_put; } + ret = dcmi_pipeline_start(dcmi); + if (ret) + goto err_media_pipeline_stop; + spin_lock_irq(&dcmi->irqlock); /* Set bus width */ @@ -676,7 +805,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) if (ret) { dev_err(dcmi->dev, "%s: Start streaming failed, cannot start capture\n", __func__); - goto err_subdev_streamoff; + goto err_pipeline_stop; } /* Enable interruptions */ @@ -687,8 +816,11 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) return 0; -err_subdev_streamoff: - v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); +err_pipeline_stop: + dcmi_pipeline_stop(dcmi); + +err_media_pipeline_stop: + media_pipeline_stop(&dcmi->vdev->entity); err_pm_put: pm_runtime_put(dcmi->dev); @@ -713,13 +845,10 @@ static void dcmi_stop_streaming(struct vb2_queue *vq) { struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq); struct dcmi_buf *buf, *node; - int ret; - /* Disable stream on the sub device */ - ret = v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0); - if (ret && ret != -ENOIOCTLCMD) - dev_err(dcmi->dev, "%s: Failed to stop streaming, subdev streamoff error (%d)\n", - __func__, ret); + dcmi_pipeline_stop(dcmi); + + media_pipeline_stop(&dcmi->vdev->entity); spin_lock_irq(&dcmi->irqlock); @@ -937,8 +1066,7 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f) mf->width = sd_framesize.width; mf->height = sd_framesize.height; - ret = v4l2_subdev_call(dcmi->entity.source, pad, - set_fmt, NULL, &format); + ret = dcmi_pipeline_s_fmt(dcmi, NULL, &format); if (ret < 0) return ret; @@ -1529,7 +1657,20 @@ static int dcmi_graph_notify_complete(struct v4l2_async_notifier *notifier) struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); int ret; + /* + * Now that the graph is complete, + * we search for the source subdevice + * in order to expose it through V4L2 interface + */ + dcmi->entity.source = + media_entity_to_v4l2_subdev(dcmi_find_source(dcmi)); + if (!dcmi->entity.source) { + dev_err(dcmi->dev, "Source subdevice not found\n"); + return -ENODEV; + } + dcmi->vdev->ctrl_handler = dcmi->entity.source->ctrl_handler; + ret = dcmi_formats_init(dcmi); if (ret) { dev_err(dcmi->dev, "No supported mediabus format found\n"); @@ -1574,12 +1715,30 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier, struct v4l2_async_subdev *asd) { struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier); + unsigned int ret; + int src_pad; dev_dbg(dcmi->dev, "Subdev %s bound\n", subdev->name); - dcmi->entity.source = subdev; + /* + * Link this sub-device to DCMI, it could be + * a parallel camera sensor or a bridge + */ + src_pad = media_entity_get_fwnode_pad(&subdev->entity, + subdev->fwnode, + MEDIA_PAD_FL_SOURCE); + + ret = media_create_pad_link(&subdev->entity, src_pad, + &dcmi->vdev->entity, 0, + MEDIA_LNK_FL_IMMUTABLE | + MEDIA_LNK_FL_ENABLED); + if (ret) + dev_err(dcmi->dev, "Failed to create media pad link with subdev %s\n", + subdev->name); + else + dev_dbg(dcmi->dev, "DCMI is now linked to %s\n", subdev->name); - return 0; + return ret; } static const struct v4l2_async_notifier_operations dcmi_graph_notify_ops = { @@ -1639,6 +1798,15 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi) return ret; } + /* Register all the subdev nodes */ + ret = v4l2_device_register_subdev_nodes(&dcmi->v4l2_dev); + if (ret) { + dev_err(dcmi->dev, "Failed to register subdev nodes\n"); + v4l2_async_notifier_unregister(&dcmi->notifier); + of_node_put(dcmi->entity.remote_node); + return ret; + } + return 0; }
Add support of several sub-devices within pipeline instead of a single one. This allows to support a CSI-2 camera sensor connected through a CSI-2 to parallel bridge. Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> --- drivers/media/platform/stm32/stm32-dcmi.c | 204 +++++++++++++++++++++++++++--- 1 file changed, 186 insertions(+), 18 deletions(-)