* [PATCH v3 1/3] media: stm32-dcmi: improve sensor subdev naming
2019-07-02 15:52 [PATCH v3 0/3] DCMI bridge support Hugues Fruchet
@ 2019-07-02 15:52 ` Hugues Fruchet
2019-07-02 15:52 ` [PATCH v3 2/3] media: stm32-dcmi: add media controller support Hugues Fruchet
2019-07-02 15:52 ` [PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices Hugues Fruchet
2 siblings, 0 replies; 8+ messages in thread
From: Hugues Fruchet @ 2019-07-02 15:52 UTC (permalink / raw)
To: Alexandre Torgue, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus
Cc: Mickael GUENE, linux-kernel, Philippe CORNU, Yannick Fertre,
Benjamin Gaignard, Hugues Fruchet, linux-stm32, linux-arm-kernel,
linux-media
Rename "subdev" entity struct field to "source"
to prepare for several subdev support.
Move asd field on top of entity struct.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
Change-Id: I1545a1a29a8061ee67cc6e4b799e9a69071911e7
---
drivers/media/platform/stm32/stm32-dcmi.c | 46 +++++++++++++++----------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index b9dad0a..b462f71 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -100,10 +100,10 @@ enum state {
#define OVERRUN_ERROR_THRESHOLD 3
struct dcmi_graph_entity {
- struct device_node *node;
-
struct v4l2_async_subdev asd;
- struct v4l2_subdev *subdev;
+
+ struct device_node *remote_node;
+ struct v4l2_subdev *source;
};
struct dcmi_format {
@@ -595,7 +595,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
}
/* Enable stream on the sub device */
- ret = v4l2_subdev_call(dcmi->entity.subdev, video, s_stream, 1);
+ 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__);
@@ -685,7 +685,7 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
return 0;
err_subdev_streamoff:
- v4l2_subdev_call(dcmi->entity.subdev, video, s_stream, 0);
+ v4l2_subdev_call(dcmi->entity.source, video, s_stream, 0);
err_pm_put:
pm_runtime_put(dcmi->dev);
@@ -713,7 +713,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
int ret;
/* Disable stream on the sub device */
- ret = v4l2_subdev_call(dcmi->entity.subdev, video, s_stream, 0);
+ 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);
@@ -857,7 +857,7 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
}
v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
- ret = v4l2_subdev_call(dcmi->entity.subdev, pad, set_fmt,
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, set_fmt,
&pad_cfg, &format);
if (ret < 0)
return ret;
@@ -934,7 +934,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.subdev, pad,
+ ret = v4l2_subdev_call(dcmi->entity.source, pad,
set_fmt, NULL, &format);
if (ret < 0)
return ret;
@@ -991,7 +991,7 @@ static int dcmi_get_sensor_format(struct stm32_dcmi *dcmi,
};
int ret;
- ret = v4l2_subdev_call(dcmi->entity.subdev, pad, get_fmt, NULL, &fmt);
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, get_fmt, NULL, &fmt);
if (ret)
return ret;
@@ -1020,7 +1020,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
}
v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
- ret = v4l2_subdev_call(dcmi->entity.subdev, pad, set_fmt,
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, set_fmt,
&pad_cfg, &format);
if (ret < 0)
return ret;
@@ -1043,7 +1043,7 @@ static int dcmi_get_sensor_bounds(struct stm32_dcmi *dcmi,
/*
* Get sensor bounds first
*/
- ret = v4l2_subdev_call(dcmi->entity.subdev, pad, get_selection,
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, get_selection,
NULL, &bounds);
if (!ret)
*r = bounds.r;
@@ -1224,7 +1224,7 @@ static int dcmi_enum_framesizes(struct file *file, void *fh,
fse.code = sd_fmt->mbus_code;
- ret = v4l2_subdev_call(dcmi->entity.subdev, pad, enum_frame_size,
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, enum_frame_size,
NULL, &fse);
if (ret)
return ret;
@@ -1241,7 +1241,7 @@ static int dcmi_g_parm(struct file *file, void *priv,
{
struct stm32_dcmi *dcmi = video_drvdata(file);
- return v4l2_g_parm_cap(video_devdata(file), dcmi->entity.subdev, p);
+ return v4l2_g_parm_cap(video_devdata(file), dcmi->entity.source, p);
}
static int dcmi_s_parm(struct file *file, void *priv,
@@ -1249,7 +1249,7 @@ static int dcmi_s_parm(struct file *file, void *priv,
{
struct stm32_dcmi *dcmi = video_drvdata(file);
- return v4l2_s_parm_cap(video_devdata(file), dcmi->entity.subdev, p);
+ return v4l2_s_parm_cap(video_devdata(file), dcmi->entity.source, p);
}
static int dcmi_enum_frameintervals(struct file *file, void *fh,
@@ -1271,7 +1271,7 @@ static int dcmi_enum_frameintervals(struct file *file, void *fh,
fie.code = sd_fmt->mbus_code;
- ret = v4l2_subdev_call(dcmi->entity.subdev, pad,
+ ret = v4l2_subdev_call(dcmi->entity.source, pad,
enum_frame_interval, NULL, &fie);
if (ret)
return ret;
@@ -1291,7 +1291,7 @@ MODULE_DEVICE_TABLE(of, stm32_dcmi_of_match);
static int dcmi_open(struct file *file)
{
struct stm32_dcmi *dcmi = video_drvdata(file);
- struct v4l2_subdev *sd = dcmi->entity.subdev;
+ struct v4l2_subdev *sd = dcmi->entity.source;
int ret;
if (mutex_lock_interruptible(&dcmi->lock))
@@ -1322,7 +1322,7 @@ static int dcmi_open(struct file *file)
static int dcmi_release(struct file *file)
{
struct stm32_dcmi *dcmi = video_drvdata(file);
- struct v4l2_subdev *sd = dcmi->entity.subdev;
+ struct v4l2_subdev *sd = dcmi->entity.source;
bool fh_singular;
int ret;
@@ -1433,7 +1433,7 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
{
const struct dcmi_format *sd_fmts[ARRAY_SIZE(dcmi_formats)];
unsigned int num_fmts = 0, i, j;
- struct v4l2_subdev *subdev = dcmi->entity.subdev;
+ struct v4l2_subdev *subdev = dcmi->entity.source;
struct v4l2_subdev_mbus_code_enum mbus_code = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
@@ -1479,7 +1479,7 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
static int dcmi_framesizes_init(struct stm32_dcmi *dcmi)
{
unsigned int num_fsize = 0;
- struct v4l2_subdev *subdev = dcmi->entity.subdev;
+ struct v4l2_subdev *subdev = dcmi->entity.source;
struct v4l2_subdev_frame_size_enum fse = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
.code = dcmi->sd_format->mbus_code,
@@ -1526,7 +1526,7 @@ static int dcmi_graph_notify_complete(struct v4l2_async_notifier *notifier)
struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier);
int ret;
- dcmi->vdev->ctrl_handler = dcmi->entity.subdev->ctrl_handler;
+ 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");
@@ -1582,7 +1582,7 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
dev_dbg(dcmi->dev, "Subdev %s bound\n", subdev->name);
- dcmi->entity.subdev = subdev;
+ dcmi->entity.source = subdev;
return 0;
}
@@ -1608,7 +1608,7 @@ static int dcmi_graph_parse(struct stm32_dcmi *dcmi, struct device_node *node)
return -EINVAL;
/* Remote node to connect */
- dcmi->entity.node = remote;
+ dcmi->entity.remote_node = remote;
dcmi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
dcmi->entity.asd.match.fwnode = of_fwnode_handle(remote);
return 0;
@@ -1631,7 +1631,7 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi)
&dcmi->entity.asd);
if (ret) {
dev_err(dcmi->dev, "Failed to add subdev notifier\n");
- of_node_put(dcmi->entity.node);
+ of_node_put(dcmi->entity.remote_node);
return ret;
}
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] media: stm32-dcmi: add media controller support
2019-07-02 15:52 [PATCH v3 0/3] DCMI bridge support Hugues Fruchet
2019-07-02 15:52 ` [PATCH v3 1/3] media: stm32-dcmi: improve sensor subdev naming Hugues Fruchet
@ 2019-07-02 15:52 ` Hugues Fruchet
2019-07-02 15:52 ` [PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices Hugues Fruchet
2 siblings, 0 replies; 8+ messages in thread
From: Hugues Fruchet @ 2019-07-02 15:52 UTC (permalink / raw)
To: Alexandre Torgue, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus
Cc: Mickael GUENE, linux-kernel, Philippe CORNU, Yannick Fertre,
Benjamin Gaignard, Hugues Fruchet, linux-stm32, linux-arm-kernel,
linux-media
Add media controller support to dcmi in order
to walk within remote subdevices pipeline.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
Change-Id: Id6280c58ea3c6f3d03da2027ac45df9f0e7a1da9
---
drivers/media/platform/Kconfig | 2 +-
drivers/media/platform/stm32/stm32-dcmi.c | 52 ++++++++++++++++++++++++-------
2 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 8a19654..de7e21f 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -121,7 +121,7 @@ config VIDEO_S3C_CAMIF
config VIDEO_STM32_DCMI
tristate "STM32 Digital Camera Memory Interface (DCMI) support"
- depends on VIDEO_V4L2 && OF
+ depends on VIDEO_V4L2 && OF && MEDIA_CONTROLLER
depends on ARCH_STM32 || COMPILE_TEST
select VIDEOBUF2_DMA_CONTIG
select V4L2_FWNODE
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index b462f71..6f37617 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -169,6 +169,9 @@ struct stm32_dcmi {
/* Ensure DMA operations atomicity */
struct mutex dma_lock;
+
+ struct media_device mdev;
+ struct media_pad vid_cap_pad;
};
static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n)
@@ -1551,14 +1554,6 @@ static int dcmi_graph_notify_complete(struct v4l2_async_notifier *notifier)
return ret;
}
- ret = video_register_device(dcmi->vdev, VFL_TYPE_GRABBER, -1);
- if (ret) {
- dev_err(dcmi->dev, "Failed to register video device\n");
- return ret;
- }
-
- dev_dbg(dcmi->dev, "Device registered as %s\n",
- video_device_node_name(dcmi->vdev));
return 0;
}
@@ -1751,10 +1746,19 @@ static int dcmi_probe(struct platform_device *pdev)
q = &dcmi->queue;
+ dcmi->v4l2_dev.mdev = &dcmi->mdev;
+
+ /* Initialize media device */
+ strscpy(dcmi->mdev.model, DRV_NAME, sizeof(dcmi->mdev.model));
+ snprintf(dcmi->mdev.bus_info, sizeof(dcmi->mdev.bus_info),
+ "platform:%s", DRV_NAME);
+ dcmi->mdev.dev = &pdev->dev;
+ media_device_init(&dcmi->mdev);
+
/* Initialize the top-level structure */
ret = v4l2_device_register(&pdev->dev, &dcmi->v4l2_dev);
if (ret)
- goto err_dma_release;
+ goto err_media_device_cleanup;
dcmi->vdev = video_device_alloc();
if (!dcmi->vdev) {
@@ -1774,6 +1778,25 @@ static int dcmi_probe(struct platform_device *pdev)
V4L2_CAP_READWRITE;
video_set_drvdata(dcmi->vdev, dcmi);
+ /* Media entity pads */
+ dcmi->vid_cap_pad.flags = MEDIA_PAD_FL_SINK;
+ ret = media_entity_pads_init(&dcmi->vdev->entity,
+ 1, &dcmi->vid_cap_pad);
+ if (ret) {
+ dev_err(dcmi->dev, "Failed to init media entity pad\n");
+ goto err_device_release;
+ }
+ dcmi->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;
+
+ ret = video_register_device(dcmi->vdev, VFL_TYPE_GRABBER, -1);
+ if (ret) {
+ dev_err(dcmi->dev, "Failed to register video device\n");
+ goto err_media_entity_cleanup;
+ }
+
+ dev_dbg(dcmi->dev, "Device registered as %s\n",
+ video_device_node_name(dcmi->vdev));
+
/* Buffer queue */
q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
q->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
@@ -1789,12 +1812,12 @@ static int dcmi_probe(struct platform_device *pdev)
ret = vb2_queue_init(q);
if (ret < 0) {
dev_err(&pdev->dev, "Failed to initialize vb2 queue\n");
- goto err_device_release;
+ goto err_media_entity_cleanup;
}
ret = dcmi_graph_init(dcmi);
if (ret < 0)
- goto err_device_release;
+ goto err_media_entity_cleanup;
/* Reset device */
ret = reset_control_assert(dcmi->rstc);
@@ -1821,11 +1844,14 @@ static int dcmi_probe(struct platform_device *pdev)
err_cleanup:
v4l2_async_notifier_cleanup(&dcmi->notifier);
+err_media_entity_cleanup:
+ media_entity_cleanup(&dcmi->vdev->entity);
err_device_release:
video_device_release(dcmi->vdev);
err_device_unregister:
v4l2_device_unregister(&dcmi->v4l2_dev);
-err_dma_release:
+err_media_device_cleanup:
+ media_device_cleanup(&dcmi->mdev);
dma_release_channel(dcmi->dma_chan);
return ret;
@@ -1839,7 +1865,9 @@ static int dcmi_remove(struct platform_device *pdev)
v4l2_async_notifier_unregister(&dcmi->notifier);
v4l2_async_notifier_cleanup(&dcmi->notifier);
+ media_entity_cleanup(&dcmi->vdev->entity);
v4l2_device_unregister(&dcmi->v4l2_dev);
+ media_device_cleanup(&dcmi->mdev);
dma_release_channel(dcmi->dma_chan);
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices
2019-07-02 15:52 [PATCH v3 0/3] DCMI bridge support Hugues Fruchet
2019-07-02 15:52 ` [PATCH v3 1/3] media: stm32-dcmi: improve sensor subdev naming Hugues Fruchet
2019-07-02 15:52 ` [PATCH v3 2/3] media: stm32-dcmi: add media controller support Hugues Fruchet
@ 2019-07-02 15:52 ` Hugues Fruchet
2019-07-25 11:40 ` Hans Verkuil
2 siblings, 1 reply; 8+ messages in thread
From: Hugues Fruchet @ 2019-07-02 15:52 UTC (permalink / raw)
To: Alexandre Torgue, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus
Cc: Mickael GUENE, linux-kernel, Philippe CORNU, Yannick Fertre,
Benjamin Gaignard, Hugues Fruchet, linux-stm32, linux-arm-kernel,
linux-media
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);
+ 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;
}
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices
2019-07-02 15:52 ` [PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices Hugues Fruchet
@ 2019-07-25 11:40 ` Hans Verkuil
2019-07-25 14:56 ` Benjamin Gaignard
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2019-07-25 11:40 UTC (permalink / raw)
To: Hugues Fruchet, Alexandre Torgue, Mauro Carvalho Chehab, Sakari Ailus
Cc: Mickael GUENE, linux-kernel, Philippe CORNU, Yannick Fertre,
Benjamin Gaignard, linux-stm32, linux-arm-kernel, linux-media
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;
> }
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices
2019-07-25 11:40 ` Hans Verkuil
@ 2019-07-25 14:56 ` Benjamin Gaignard
2019-07-25 15:09 ` Hans Verkuil
0 siblings, 1 reply; 8+ messages in thread
From: Benjamin Gaignard @ 2019-07-25 14:56 UTC (permalink / raw)
To: Hans Verkuil
Cc: Alexandre Torgue, Mickael GUENE, Linux Kernel Mailing List,
Philippe CORNU, Yannick Fertre, Sakari Ailus, Hugues Fruchet,
Mauro Carvalho Chehab, linux-stm32, Linux ARM, linux-media
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;
> > }
> >
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices
2019-07-25 14:56 ` Benjamin Gaignard
@ 2019-07-25 15:09 ` Hans Verkuil
2019-07-31 13:05 ` Hugues FRUCHET
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2019-07-25 15:09 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: Alexandre Torgue, Mickael GUENE, Linux Kernel Mailing List,
Philippe CORNU, Yannick Fertre, Sakari Ailus, Hugues Fruchet,
Mauro Carvalho Chehab, linux-stm32, Linux ARM, linux-media
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;
>>> }
>>>
>>>
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices
2019-07-25 15:09 ` Hans Verkuil
@ 2019-07-31 13:05 ` Hugues FRUCHET
0 siblings, 0 replies; 8+ messages in thread
From: Hugues FRUCHET @ 2019-07-31 13:05 UTC (permalink / raw)
To: Hans Verkuil, Benjamin Gaignard
Cc: Alexandre TORGUE, Mickael GUENE, Linux Kernel Mailing List,
Philippe CORNU, Yannick FERTRE, Sakari Ailus,
Mauro Carvalho Chehab, linux-stm32, Linux ARM, linux-media
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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread