2019-08-19 08:42:55

by Hugues Fruchet

[permalink] [raw]
Subject: [PATCH v7 0/4] DCMI bridge support

This patch serie allows to connect non-parallel camera sensor to
DCMI thanks to a bridge connected in between such as STMIPID02 [1].

Media controller support is introduced first, then support of
several sub-devices within pipeline with dynamic linking
between them.
In order to keep backward compatibility with applications
relying on V4L2 interface only, format set on video node
is propagated to all sub-devices connected to camera interface.

[1] https://www.spinics.net/lists/devicetree/msg278002.html

===========
= history =
===========
version 7:
- minor fix on 80 char trace message

version 6:
- As per Sakari remark: add a FIXME explaining that this
version only supports subdevices which expose RGB & YUV
"parallel form" mbus code (_2X8)
- Add some trace around subdev_call(s_fmt) error & format
changes to debug subdev which only expose serial mbus code
- Conform to "<name>":<pad index> when tracing subdev infos

version 5:
- Remove remaining Change-Id
- Add Acked-by: Sakari Ailus <[email protected]>

version 4:
- Also drop subdev nodes registry as suggested by Hans:
https://www.spinics.net/lists/arm-kernel/msg743375.html

version 3:
- Drop media device registry to not expose media controller
interface to userspace as per Laurent' suggestion:
https://www.spinics.net/lists/linux-media/msg153417.html
- Prefer "source" instead of "sensor" and keep it in
dcmi_graph_entity struct, move asd as first member
of struct as per Sakari' suggestion:
https://www.spinics.net/lists/linux-media/msg153119.html
- Drop dcmi_graph_deinit() as per Sakari' suggestion:
https://www.spinics.net/lists/linux-media/msg153417.html

version 2:
- Fix bus_info not consistent between media and V4L:
https://www.spinics.net/lists/arm-kernel/msg717676.html
- Propagation of format set on video node to the sub-devices
chain connected on camera interface

version 1:
- Initial submission

Hugues Fruchet (4):
media: stm32-dcmi: improve sensor subdev naming
media: stm32-dcmi: trace the supported fourcc/mbus_code
media: stm32-dcmi: add media controller support
media: stm32-dcmi: add support of several sub-devices

drivers/media/platform/Kconfig | 2 +-
drivers/media/platform/stm32/stm32-dcmi.c | 318 +++++++++++++++++++++++++-----
2 files changed, 268 insertions(+), 52 deletions(-)

--
2.7.4


2019-08-19 08:42:57

by Hugues Fruchet

[permalink] [raw]
Subject: [PATCH v7 1/4] media: stm32-dcmi: improve sensor subdev naming

Rename "subdev" entity struct field to "source"
to prepare for several subdev support.
Move asd field on top of entity struct.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Hugues Fruchet <[email protected]>
---
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

2019-08-19 08:44:01

by Hugues Fruchet

[permalink] [raw]
Subject: [PATCH v7 2/4] media: stm32-dcmi: trace the supported fourcc/mbus_code

Add a trace of the set of supported fourcc/mbus_code which
intersect between DCMI and source sub-device.

Signed-off-by: Hugues Fruchet <[email protected]>
---
drivers/media/platform/stm32/stm32-dcmi.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index b462f71..db01210 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1447,12 +1447,21 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
/* Code supported, have we got this fourcc yet? */
for (j = 0; j < num_fmts; j++)
if (sd_fmts[j]->fourcc ==
- dcmi_formats[i].fourcc)
+ dcmi_formats[i].fourcc) {
/* Already available */
+ dev_dbg(dcmi->dev, "Skipping fourcc/code: %4.4s/0x%x\n",
+ (char *)&sd_fmts[j]->fourcc,
+ mbus_code.code);
break;
- if (j == num_fmts)
+ }
+ if (j == num_fmts) {
/* New */
sd_fmts[num_fmts++] = dcmi_formats + i;
+ dev_dbg(dcmi->dev,
+ "Supported fourcc/code: %4.4s/0x%x\n",
+ (char *)&sd_fmts[num_fmts - 1]->fourcc,
+ sd_fmts[num_fmts - 1]->mbus_code);
+ }
}
mbus_code.index++;
}
--
2.7.4

2019-08-19 08:44:30

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] DCMI bridge support

On 8/19/19 10:41 AM, Hugues Fruchet wrote:
> This patch serie allows to connect non-parallel camera sensor to
> DCMI thanks to a bridge connected in between such as STMIPID02 [1].
>
> Media controller support is introduced first, then support of
> several sub-devices within pipeline with dynamic linking
> between them.
> In order to keep backward compatibility with applications
> relying on V4L2 interface only, format set on video node
> is propagated to all sub-devices connected to camera interface.
>
> [1] https://www.spinics.net/lists/devicetree/msg278002.html
>
> ===========
> = history =
> ===========
> version 7:
> - minor fix on 80 char trace message

v6 is already in a pending PR. I don't really want to make a new
PR just for a 80 char warning.

It can always be done in a follow-up patch.

Regards,

Hans

>
> version 6:
> - As per Sakari remark: add a FIXME explaining that this
> version only supports subdevices which expose RGB & YUV
> "parallel form" mbus code (_2X8)
> - Add some trace around subdev_call(s_fmt) error & format
> changes to debug subdev which only expose serial mbus code
> - Conform to "<name>":<pad index> when tracing subdev infos
>
> version 5:
> - Remove remaining Change-Id
> - Add Acked-by: Sakari Ailus <[email protected]>
>
> version 4:
> - Also drop subdev nodes registry as suggested by Hans:
> https://www.spinics.net/lists/arm-kernel/msg743375.html
>
> version 3:
> - Drop media device registry to not expose media controller
> interface to userspace as per Laurent' suggestion:
> https://www.spinics.net/lists/linux-media/msg153417.html
> - Prefer "source" instead of "sensor" and keep it in
> dcmi_graph_entity struct, move asd as first member
> of struct as per Sakari' suggestion:
> https://www.spinics.net/lists/linux-media/msg153119.html
> - Drop dcmi_graph_deinit() as per Sakari' suggestion:
> https://www.spinics.net/lists/linux-media/msg153417.html
>
> version 2:
> - Fix bus_info not consistent between media and V4L:
> https://www.spinics.net/lists/arm-kernel/msg717676.html
> - Propagation of format set on video node to the sub-devices
> chain connected on camera interface
>
> version 1:
> - Initial submission
>
> Hugues Fruchet (4):
> media: stm32-dcmi: improve sensor subdev naming
> media: stm32-dcmi: trace the supported fourcc/mbus_code
> media: stm32-dcmi: add media controller support
> media: stm32-dcmi: add support of several sub-devices
>
> drivers/media/platform/Kconfig | 2 +-
> drivers/media/platform/stm32/stm32-dcmi.c | 318 +++++++++++++++++++++++++-----
> 2 files changed, 268 insertions(+), 52 deletions(-)
>

2019-08-19 09:15:03

by Hugues Fruchet

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] DCMI bridge support

Hi Hans, Sakari,

OK to push separately the 80 char fix.

There was pending related changes on st-mipid02 and ov5640 (listed
below), do you think it's possible to take them also ?


media: st-mipid02: add support of V4L2_CID_LINK_FREQ
https://patchwork.linuxtv.org/patch/56969/
State Accepted

[v2,1/3] media: st-mipid02: add support of RGB565
https://patchwork.linuxtv.org/patch/56970/
State Accepted

[v2,2/3] media: st-mipid02: add support of YUYV8 and UYVY8
https://patchwork.linuxtv.org/patch/56971/
State Accepted

[v2,3/3] media: st-mipid02: add support of JPEG
https://patchwork.linuxtv.org/patch/56973/
State Accepted


[v2] media: ov5640: add support of V4L2_CID_LINK_FREQ
https://patchwork.linuxtv.org/patch/57215/
State Changes Requested
=> This change is needed to make it work the whole setup.
=> I don't know what to change here, even if this 384MHz fixed value
seems strange, it works fine on my setup, on my opinion it's better than
nothing. We could come back on this later on when other OV5640 CSI
interfaces will require V4L2_CID_LINK_FREQ value.

Sakari, what do you think about this ?


BR,
Hugues.

On 8/19/19 10:43 AM, Hans Verkuil wrote:
> On 8/19/19 10:41 AM, Hugues Fruchet wrote:
>> This patch serie allows to connect non-parallel camera sensor to
>> DCMI thanks to a bridge connected in between such as STMIPID02 [1].
>>
>> Media controller support is introduced first, then support of
>> several sub-devices within pipeline with dynamic linking
>> between them.
>> In order to keep backward compatibility with applications
>> relying on V4L2 interface only, format set on video node
>> is propagated to all sub-devices connected to camera interface.
>>
>> [1] https://www.spinics.net/lists/devicetree/msg278002.html
>>
>> ===========
>> = history =
>> ===========
>> version 7:
>> - minor fix on 80 char trace message
>
> v6 is already in a pending PR. I don't really want to make a new
> PR just for a 80 char warning.
>
> It can always be done in a follow-up patch.
>
> Regards,
>
> Hans
>
>>
>> version 6:
>> - As per Sakari remark: add a FIXME explaining that this
>> version only supports subdevices which expose RGB & YUV
>> "parallel form" mbus code (_2X8)
>> - Add some trace around subdev_call(s_fmt) error & format
>> changes to debug subdev which only expose serial mbus code
>> - Conform to "<name>":<pad index> when tracing subdev infos
>>
>> version 5:
>> - Remove remaining Change-Id
>> - Add Acked-by: Sakari Ailus <[email protected]>
>>
>> version 4:
>> - Also drop subdev nodes registry as suggested by Hans:
>> https://www.spinics.net/lists/arm-kernel/msg743375.html
>>
>> version 3:
>> - Drop media device registry to not expose media controller
>> interface to userspace as per Laurent' suggestion:
>> https://www.spinics.net/lists/linux-media/msg153417.html
>> - Prefer "source" instead of "sensor" and keep it in
>> dcmi_graph_entity struct, move asd as first member
>> of struct as per Sakari' suggestion:
>> https://www.spinics.net/lists/linux-media/msg153119.html
>> - Drop dcmi_graph_deinit() as per Sakari' suggestion:
>> https://www.spinics.net/lists/linux-media/msg153417.html
>>
>> version 2:
>> - Fix bus_info not consistent between media and V4L:
>> https://www.spinics.net/lists/arm-kernel/msg717676.html
>> - Propagation of format set on video node to the sub-devices
>> chain connected on camera interface
>>
>> version 1:
>> - Initial submission
>>
>> Hugues Fruchet (4):
>> media: stm32-dcmi: improve sensor subdev naming
>> media: stm32-dcmi: trace the supported fourcc/mbus_code
>> media: stm32-dcmi: add media controller support
>> media: stm32-dcmi: add support of several sub-devices
>>
>> drivers/media/platform/Kconfig | 2 +-
>> drivers/media/platform/stm32/stm32-dcmi.c | 318 +++++++++++++++++++++++++-----
>> 2 files changed, 268 insertions(+), 52 deletions(-)
>>
>

2019-08-20 08:11:35

by Hugues Fruchet

[permalink] [raw]
Subject: Re: [PATCH v7 0/4] DCMI bridge support

Hi Sakari, Hans,

st-mipid02 changes are already merged, thanks Sakari and sorry for
disturbance.

Still remain the V4L2_CID_LINK_FREQ for OV5640.


On 8/19/19 11:13 AM, Hugues FRUCHET wrote:
> Hi Hans, Sakari,
>
> OK to push separately the 80 char fix.
>
> There was pending related changes on st-mipid02 and ov5640 (listed
> below), do you think it's possible to take them also ?
>
>
> media: st-mipid02: add support of V4L2_CID_LINK_FREQ
> https://patchwork.linuxtv.org/patch/56969/
> State    Accepted
>
> [v2,1/3] media: st-mipid02: add support of RGB565
> https://patchwork.linuxtv.org/patch/56970/
> State    Accepted
>
> [v2,2/3] media: st-mipid02: add support of YUYV8 and UYVY8
> https://patchwork.linuxtv.org/patch/56971/
> State    Accepted
>
> [v2,3/3] media: st-mipid02: add support of JPEG
> https://patchwork.linuxtv.org/patch/56973/
> State    Accepted
>
>
> [v2] media: ov5640: add support of V4L2_CID_LINK_FREQ
> https://patchwork.linuxtv.org/patch/57215/
> State    Changes Requested
> => This change is needed to make it work the whole setup.
> => I don't know what to change here, even if this 384MHz fixed value
> seems strange, it works fine on my setup, on my opinion it's better than
> nothing. We could come back on this later on when other OV5640 CSI
> interfaces will require V4L2_CID_LINK_FREQ value.
>
> Sakari, what do you think about this ?
>
>
> BR,
> Hugues.

BR,
Hugues.