[PATCH v5 1/4] media: stm32-dcmi: add support of BT656 bus
Add support of BT656 embedded synchronization bus.
This mode allows to save hardware synchro lines hsync & vsync by replacing
them with synchro codes embedded in data stream.
This bus type is only compatible with 8 bits width data bus.
Due to reserved values 0x00 & 0xff used for synchro codes, valid data vary
from 0x1 to 0xfe, this is up to sensor to clip accordingly pixel data. As
a consequence of this clipping, JPEG is not supported with this bus type.
DCMI crop feature is also not available with this bus type.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 37 +--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index fd1c41c..7705683 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -157,6 +157,7 @@ struct stm32_dcmi {
struct vb2_queuequeue;
struct v4l2_fwnode_bus_parallel bus;
+ enum v4l2_mbus_type bus_type;
struct completion complete;
struct clk *mclk;
enum state state;
@@ -777,6 +778,23 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
if (dcmi->bus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
val |= CR_PCKPOL;
+ /*
+* BT656 embedded synchronisation bus mode.
+*
+* Default SAV/EAV mode is supported here with default codes
+* SAV=0xff80 & EAV=0xff9d.
+* With DCMI this means LSC=SAV=0x80 & LEC=EAV=0x9d.
+*/
+ if (dcmi->bus_type == V4L2_MBUS_BT656) {
+ val |= CR_ESS;
+
+ /* Unmask all codes */
+ reg_write(dcmi->regs, DCMI_ESUR, 0x);/* FEC:LEC:LSC:FSC
*/
+
+ /* Trig on LSC=0x80 & LEC=0x9d codes, ignore FSC and FEC */
+ reg_write(dcmi->regs, DCMI_ESCR, 0xff9d80ff);/* FEC:LEC:LSC:FSC
*/
+ }
+
reg_write(dcmi->regs, DCMI_CR, val);
/* Set crop */
@@ -1067,8 +1085,9 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct
v4l2_format *f)
if (ret)
return ret;
- /* Disable crop if JPEG is requested */
- if (pix->pixelformat == V4L2_PIX_FMT_JPEG)
+ /* Disable crop if JPEG is requested or BT656 bus is selected */
+ if (pix->pixelformat == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type != V4L2_MBUS_BT656)
dcmi->do_crop = false;
/* pix to mbus format */
@@ -1592,6 +1611,11 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
if (dcmi_formats[i].mbus_code != mbus_code.code)
continue;
+ /* Exclude JPEG if BT656 bus is selected */
+ if (dcmi_formats[i].fourcc == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type == V4L2_MBUS_BT656)
+ continue;
+
/* Code supported, have we got this fourcc yet? */
for (j = 0; j < num_fmts; j++)
if (sd_fmts[j]->fourcc ==
@@ -1873,9 +1897,18 @@ static int dcmi_probe(struct platform_device *pdev)
dev_err(>dev, "CSI bus not supported\n");
return -ENODEV;
}
+
+ if (ep.bus_type == V4L2_MBUS_BT656 &&
+ ep.bus.parallel.bus_width != 8) {
+ dev_err(>dev, "BT656 bus conflicts with %u bits bus width
(8 bits required)\n",
+ ep.bus.parallel.bus_width);
+ return -ENODEV;
+ }
+
dcmi->bus.flags = ep.bus.parallel.flags;
dcmi->bus.bus_width = ep.bus.parallel.bus_width;
dcmi->bus.data_shift = ep.bus.parallel.data_shift;
+ dcmi->bus_type = ep.bus_type;
irq = platform_get_irq(pdev, 0);
if (irq <= 0)
--
2.7.4
[PATCH v5 3/4] ARM: dts: stm32: set bus-type in DCMI endpoint for stm32mp157c-ev1 board
Explicitly set bus-type to parallel mode in DCMI endpoint (bus-type=5).
Signed-off-by: Hugues Fruchet
---
arch/arm/boot/dts/stm32mp157c-ev1.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts
b/arch/arm/boot/dts/stm32mp157c-ev1.dts
index 85628e1..6cc5d2a 100644
--- a/arch/arm/boot/dts/stm32mp157c-ev1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts
@@ -90,6 +90,7 @@
port {
dcmi_0: endpoint {
remote-endpoint = <_0>;
+ bus-type = <5>;
bus-width = <8>;
hsync-active = <0>;
vsync-active = <0>;
--
2.7.4
[PATCH v5 2/4] media: dt-bindings: media: st,stm32-dcmi: add support of BT656 bus
Add support of BT656 embedded synchronization bus mode in DCMI driver.
Add "bus-type" property and make it required so that there is no
ambiguity between parallel mode (bus-type=5) and BT656 mode (bus-type=6).
BT656 mode allows to save hardware synchro lines hsync & vsync by replacing
them with synchro codes embedded in data stream, hence hsync-active &
vsync-active properties are useless in this mode.
With DCMI, BT656 bus mode is only compatible with 8 bits width data bus.
Signed-off-by: Hugues Fruchet
---
.../devicetree/bindings/media/st,stm32-dcmi.yaml | 38 ++
1 file changed, 38 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
index 3fe778c..c18574b 100644
--- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
+++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
@@ -44,6 +44,43 @@ properties:
bindings defined in
Documentation/devicetree/bindings/media/video-interfaces.txt.
+properties:
+ endpoint:
+type: object
+
+properties:
+ bus-type:
+enum: [5, 6]
+default: 5
+
+ bus-width:
+enum: [8, 10, 12, 14]
+default: 8
+
+ remote-endpoint: true
+
+allOf:
+ - if:
+ properties:
+bus-type:
+ const: 6
+
+then:
+ properties:
+hsync-active: false
+vsync-active: false
+bus-width:
+ enum: [8]
+
+required:
+ - remote-endpoint
+ - bus-type
+ - pclk-sample
+
+unevaluatedProperties: false
+
+additionalProperties: false
+
required:
- compatible
- reg
@@ -75,6 +112,7 @@ examples:
port {
dcmi_0: endpoint {
remote-endpoint = <_0>;
+ bus-type = <5>;
bus-width = <8>;
hsync-active = <0>;
vsync-active = <0>;
--
2.7.4
[PATCH v5 4/4] ARM: dts: stm32: set bus-type in DCMI endpoint for stm32429i-eval board
Explicitly set bus-type to parallel mode in DCMI endpoint (bus-type=5).
Signed-off-by: Hugues Fruchet
---
arch/arm/boot/dts/stm32429i-eval.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
b/arch/arm/boot/dts/stm32429i-eval.dts
index 67e7648..7e10ae7 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -188,6 +188,7 @@
port {
dcmi_0: endpoint {
remote-endpoint = <_0>;
+ bus-type = <5>;
bus-width = <8>;
hsync-active = <0>;
vsync-active = <0>;
--
2.7.4
[PATCH v5 0/4] DCMI BT656 parallel bus mode support
Add support of BT656 embedded synchronization bus. This mode allows to save hardware synchro lines hsync & vsync by replacing them with synchro codes embedded in data stream. Add "bus-type" property and make it required so that there is no ambiguity between parallel mode (bus-type=5) and BT656 mode (bus-type=6). === = history = === version 5: - Add revisited bindings and devicetree with explicit use of "bus-type" version 4: - Fix typo in commit message version 3: - Fix bus_width print to %u as per Sakari comment version 2: - As per Sakari remark, revisit commit message and document BT656 parallel bus mode in bindings version 1: - Initial submission Hugues Fruchet (4): media: stm32-dcmi: add support of BT656 bus media: dt-bindings: media: st,stm32-dcmi: add support of BT656 bus ARM: dts: stm32: set bus-type in DCMI endpoint for stm32mp157c-ev1 board ARM: dts: stm32: set bus-type in DCMI endpoint for stm32429i-eval board .../devicetree/bindings/media/st,stm32-dcmi.yaml | 38 ++ arch/arm/boot/dts/stm32429i-eval.dts | 1 + arch/arm/boot/dts/stm32mp157c-ev1.dts | 1 + drivers/media/platform/stm32/stm32-dcmi.c | 37 +++-- 4 files changed, 75 insertions(+), 2 deletions(-) -- 2.7.4
Re: [PATCH v4 2/2] media: dt-bindings: media: st,stm32-dcmi: Add support of BT656
Hi Sakari, + Jacopo for his work on ov772x binding related to BT656 On 10/21/20 11:40 PM, Sakari Ailus wrote: > Hi Hugues, > > On Wed, Oct 21, 2020 at 02:24:08PM +0000, Hugues FRUCHET wrote: >> Hi Sakari, >> >> On 10/21/20 3:00 PM, Sakari Ailus wrote: >>> Hi Hugues, >>> >>> On Tue, Oct 20, 2020 at 12:14:49PM +0200, Hugues Fruchet wrote: >>>> Add support of BT656 parallel bus mode in DCMI. >>>> This mode is enabled when hsync-active & vsync-active >>>> fields are not specified. >>>> >>>> Signed-off-by: Hugues Fruchet >>>> --- >>>>.../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 >>>> ++ >>>>1 file changed, 30 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >>>> b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >>>> index 3fe778c..1ee521a 100644 >>>> --- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >>>> +++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >>>> @@ -44,6 +44,36 @@ properties: >>>> bindings defined in >>>> Documentation/devicetree/bindings/media/video-interfaces.txt. >>>> >>>> +properties: >>>> + endpoint: >>>> +type: object >>>> + >>>> +properties: >>>> + bus-width: true >>>> + >>>> + hsync-active: >>>> +description: >>>> + If both HSYNC and VSYNC polarities are not specified, BT656 >>>> + embedded synchronization is selected. >>>> +default: 0 >>>> + >>>> + vsync-active: >>>> +description: >>>> + If both HSYNC and VSYNC polarities are not specified, BT656 >>>> + embedded synchronization is selected. >>>> +default: 0 >>> >>> Should I understand this as if the polarities were not specified, BT.656 >>> will be used? >> >> Yes, this is what is documented in video-interfaces.txt: >> " >> Note, that if HSYNC and VSYNC polarities are not specified, embedded >> synchronization may be required, where supported. >> " >> and >> " >> /* If hsync-active/vsync-active are missing, >> embedded BT.656 sync is used */ >> hsync-active = <0>; /* Active low */ >> vsync-active = <0>; /* Active low */ >> " >> and I found also this in >> Documentation/devicetree/bindings/media/renesas,vin.yaml >> " >> hsync-active: >> description: >> If both HSYNC and VSYNC polarities are not specified, >> embedded >> synchronization is selected. >> default: 1 >> >> vsync-active: >> description: >> If both HSYNC and VSYNC polarities are not specified, >> embedded >> synchronization is selected. >> default: 1 > > Having the defaults leads to somewhat weird behaviour: specifying the > default value on either property changes the bus type. > >> " >> >> In the other hand I've found few occurences of "bus-type" >> (marvell,mmp2-ccic.yaml), it is why I asked you if "bus-type" is the new >> way to go versus previous way to signal BT656 (without hsync/vsync) ? >> As explained previously, I prefer this last way for backward compatibility. > > If you have a default for bus-type (BT.601), this won't be a problem. > > The old DT bindings were somewhat, well, opportunistic. The v4l2-of > framework-let did its best and sometimes it worked. The behaviour is still > supported but not encouraged in new bindings. > OK, so let's go for the new way. I've found an interesting patch from Jacopo that is of great help: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20200910162055.614089-4-jacopo+rene...@jmondi.org/ Here is a draft proposal before I push a new version, please comment: properties: bus-type: enum: [5, 6] default: 5 bus-width: enum: [8, 10, 12, 14] default: 8 hsync-active: enum: [0, 1] default: 0 vsy
Re: [PATCH v4 2/2] media: dt-bindings: media: st,stm32-dcmi: Add support of BT656
Hi Sakari, On 10/21/20 3:00 PM, Sakari Ailus wrote: > Hi Hugues, > > On Tue, Oct 20, 2020 at 12:14:49PM +0200, Hugues Fruchet wrote: >> Add support of BT656 parallel bus mode in DCMI. >> This mode is enabled when hsync-active & vsync-active >> fields are not specified. >> >> Signed-off-by: Hugues Fruchet >> --- >> .../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 >> ++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >> b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >> index 3fe778c..1ee521a 100644 >> --- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >> +++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >> @@ -44,6 +44,36 @@ properties: >> bindings defined in >> Documentation/devicetree/bindings/media/video-interfaces.txt. >> >> +properties: >> + endpoint: >> +type: object >> + >> +properties: >> + bus-width: true >> + >> + hsync-active: >> +description: >> + If both HSYNC and VSYNC polarities are not specified, BT656 >> + embedded synchronization is selected. >> +default: 0 >> + >> + vsync-active: >> +description: >> + If both HSYNC and VSYNC polarities are not specified, BT656 >> + embedded synchronization is selected. >> +default: 0 > > Should I understand this as if the polarities were not specified, BT.656 > will be used? Yes, this is what is documented in video-interfaces.txt: " Note, that if HSYNC and VSYNC polarities are not specified, embedded synchronization may be required, where supported. " and " /* If hsync-active/vsync-active are missing, embedded BT.656 sync is used */ hsync-active = <0>; /* Active low */ vsync-active = <0>; /* Active low */ " and I found also this in Documentation/devicetree/bindings/media/renesas,vin.yaml " hsync-active: description: If both HSYNC and VSYNC polarities are not specified, embedded synchronization is selected. default: 1 vsync-active: description: If both HSYNC and VSYNC polarities are not specified, embedded synchronization is selected. default: 1 " In the other hand I've found few occurences of "bus-type" (marvell,mmp2-ccic.yaml), it is why I asked you if "bus-type" is the new way to go versus previous way to signal BT656 (without hsync/vsync) ? As explained previously, I prefer this last way for backward compatibility. The bindings previously documented BT.601 (parallel) only, so > it was somewhat ambigious to begin with. Is there a risk of interpreting > old BT.601 bindings as BT.656? I don't think so. With bus-type property, I believe you could > avoid at least that risk. yes but as explained, I'll prefer not to amend current boards device tree files. > > Also not specifying at least one of the default values leads to BT.656 > without bus-type. That could be addressed by removing the defaults. > I'm new to yaml, I've taken that from renesas,vin.yaml. Should I just drop the "default: 1" lines ? >> + >> + pclk-sample: true >> + >> + remote-endpoint: true >> + >> +required: >> + - remote-endpoint >> + >> +additionalProperties: false >> + >> +additionalProperties: false >> + >> required: >> - compatible >> - reg > BR, Hugues.
[PATCH v4 2/2] media: dt-bindings: media: st,stm32-dcmi: Add support of BT656
Add support of BT656 parallel bus mode in DCMI. This mode is enabled when hsync-active & vsync-active fields are not specified. Signed-off-by: Hugues Fruchet --- .../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 ++ 1 file changed, 30 insertions(+) diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml index 3fe778c..1ee521a 100644 --- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml +++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml @@ -44,6 +44,36 @@ properties: bindings defined in Documentation/devicetree/bindings/media/video-interfaces.txt. +properties: + endpoint: +type: object + +properties: + bus-width: true + + hsync-active: +description: + If both HSYNC and VSYNC polarities are not specified, BT656 + embedded synchronization is selected. +default: 0 + + vsync-active: +description: + If both HSYNC and VSYNC polarities are not specified, BT656 + embedded synchronization is selected. +default: 0 + + pclk-sample: true + + remote-endpoint: true + +required: + - remote-endpoint + +additionalProperties: false + +additionalProperties: false + required: - compatible - reg -- 2.7.4
[PATCH v4 1/2] media: stm32-dcmi: add support of BT656 bus
Add support of BT656 embedded synchronization bus.
This mode allows to save hardware synchro lines hsync & vsync by replacing
them with synchro codes embedded in data stream.
This bus type is only compatible with 8 bits width data bus.
Due to reserved values 0x00 & 0xff used for synchro codes, valid data vary
from 0x1 to 0xfe, this is up to sensor to clip accordingly pixel data. As
a consequence of this clipping, JPEG is not supported with this bus type.
DCMI crop feature is also not available with this bus type.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 37 +--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index fd1c41c..7705683 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -157,6 +157,7 @@ struct stm32_dcmi {
struct vb2_queuequeue;
struct v4l2_fwnode_bus_parallel bus;
+ enum v4l2_mbus_type bus_type;
struct completion complete;
struct clk *mclk;
enum state state;
@@ -777,6 +778,23 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
if (dcmi->bus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
val |= CR_PCKPOL;
+ /*
+* BT656 embedded synchronisation bus mode.
+*
+* Default SAV/EAV mode is supported here with default codes
+* SAV=0xff80 & EAV=0xff9d.
+* With DCMI this means LSC=SAV=0x80 & LEC=EAV=0x9d.
+*/
+ if (dcmi->bus_type == V4L2_MBUS_BT656) {
+ val |= CR_ESS;
+
+ /* Unmask all codes */
+ reg_write(dcmi->regs, DCMI_ESUR, 0x);/* FEC:LEC:LSC:FSC
*/
+
+ /* Trig on LSC=0x80 & LEC=0x9d codes, ignore FSC and FEC */
+ reg_write(dcmi->regs, DCMI_ESCR, 0xff9d80ff);/* FEC:LEC:LSC:FSC
*/
+ }
+
reg_write(dcmi->regs, DCMI_CR, val);
/* Set crop */
@@ -1067,8 +1085,9 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct
v4l2_format *f)
if (ret)
return ret;
- /* Disable crop if JPEG is requested */
- if (pix->pixelformat == V4L2_PIX_FMT_JPEG)
+ /* Disable crop if JPEG is requested or BT656 bus is selected */
+ if (pix->pixelformat == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type != V4L2_MBUS_BT656)
dcmi->do_crop = false;
/* pix to mbus format */
@@ -1592,6 +1611,11 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
if (dcmi_formats[i].mbus_code != mbus_code.code)
continue;
+ /* Exclude JPEG if BT656 bus is selected */
+ if (dcmi_formats[i].fourcc == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type == V4L2_MBUS_BT656)
+ continue;
+
/* Code supported, have we got this fourcc yet? */
for (j = 0; j < num_fmts; j++)
if (sd_fmts[j]->fourcc ==
@@ -1873,9 +1897,18 @@ static int dcmi_probe(struct platform_device *pdev)
dev_err(>dev, "CSI bus not supported\n");
return -ENODEV;
}
+
+ if (ep.bus_type == V4L2_MBUS_BT656 &&
+ ep.bus.parallel.bus_width != 8) {
+ dev_err(>dev, "BT656 bus conflicts with %u bits bus width
(8 bits required)\n",
+ ep.bus.parallel.bus_width);
+ return -ENODEV;
+ }
+
dcmi->bus.flags = ep.bus.parallel.flags;
dcmi->bus.bus_width = ep.bus.parallel.bus_width;
dcmi->bus.data_shift = ep.bus.parallel.data_shift;
+ dcmi->bus_type = ep.bus_type;
irq = platform_get_irq(pdev, 0);
if (irq <= 0)
--
2.7.4
[PATCH v4 0/2] DCMI BT656 parallel bus mode support
Add support of BT656 embedded synchronization bus. This mode allows to save hardware synchro lines hsync & vsync by replacing them with synchro codes embedded in data stream. This mode is enabled when hsync-active & vsync-active fields are not specified in devicetree. === = history = === version 4: - Fix typo in commit message version 3: - Fix bus_width print to %u as per Sakari comment version 2: - As per Sakari remark, revisit commit message and document BT656 parallel bus mode in bindings version 1: - Initial submission Hugues Fruchet (2): media: stm32-dcmi: add support of BT656 bus media: dt-bindings: media: st,stm32-dcmi: Add support of BT656 .../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 ++ drivers/media/platform/stm32/stm32-dcmi.c | 37 -- 2 files changed, 65 insertions(+), 2 deletions(-) -- 2.7.4
[PATCH v3 1/2] media: stm32-dcmi: add support of BT656 bus
Add support of BT656 embedded synchronization busa
Add support of BT656 embedded synchronization bus.
This mode allows to save hardware synchro lines hsync & vsync by replacing
them with synchro codes embedded in data stream.
This bus type is only compatible with 8 bits width data bus.
Due to reserved values 0x00 & 0xff used for synchro codes, valid data vary
from 0x1 to 0xfe, this is up to sensor to clip accordingly pixel data. As
a consequence of this clipping, JPEG is not supported with this bus type.
DCMI crop feature is also not available with this bus type.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 37 +--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index fd1c41c..7705683 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -157,6 +157,7 @@ struct stm32_dcmi {
struct vb2_queuequeue;
struct v4l2_fwnode_bus_parallel bus;
+ enum v4l2_mbus_type bus_type;
struct completion complete;
struct clk *mclk;
enum state state;
@@ -777,6 +778,23 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
if (dcmi->bus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
val |= CR_PCKPOL;
+ /*
+* BT656 embedded synchronisation bus mode.
+*
+* Default SAV/EAV mode is supported here with default codes
+* SAV=0xff80 & EAV=0xff9d.
+* With DCMI this means LSC=SAV=0x80 & LEC=EAV=0x9d.
+*/
+ if (dcmi->bus_type == V4L2_MBUS_BT656) {
+ val |= CR_ESS;
+
+ /* Unmask all codes */
+ reg_write(dcmi->regs, DCMI_ESUR, 0x);/* FEC:LEC:LSC:FSC
*/
+
+ /* Trig on LSC=0x80 & LEC=0x9d codes, ignore FSC and FEC */
+ reg_write(dcmi->regs, DCMI_ESCR, 0xff9d80ff);/* FEC:LEC:LSC:FSC
*/
+ }
+
reg_write(dcmi->regs, DCMI_CR, val);
/* Set crop */
@@ -1067,8 +1085,9 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct
v4l2_format *f)
if (ret)
return ret;
- /* Disable crop if JPEG is requested */
- if (pix->pixelformat == V4L2_PIX_FMT_JPEG)
+ /* Disable crop if JPEG is requested or BT656 bus is selected */
+ if (pix->pixelformat == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type != V4L2_MBUS_BT656)
dcmi->do_crop = false;
/* pix to mbus format */
@@ -1592,6 +1611,11 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
if (dcmi_formats[i].mbus_code != mbus_code.code)
continue;
+ /* Exclude JPEG if BT656 bus is selected */
+ if (dcmi_formats[i].fourcc == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type == V4L2_MBUS_BT656)
+ continue;
+
/* Code supported, have we got this fourcc yet? */
for (j = 0; j < num_fmts; j++)
if (sd_fmts[j]->fourcc ==
@@ -1873,9 +1897,18 @@ static int dcmi_probe(struct platform_device *pdev)
dev_err(>dev, "CSI bus not supported\n");
return -ENODEV;
}
+
+ if (ep.bus_type == V4L2_MBUS_BT656 &&
+ ep.bus.parallel.bus_width != 8) {
+ dev_err(>dev, "BT656 bus conflicts with %u bits bus width
(8 bits required)\n",
+ ep.bus.parallel.bus_width);
+ return -ENODEV;
+ }
+
dcmi->bus.flags = ep.bus.parallel.flags;
dcmi->bus.bus_width = ep.bus.parallel.bus_width;
dcmi->bus.data_shift = ep.bus.parallel.data_shift;
+ dcmi->bus_type = ep.bus_type;
irq = platform_get_irq(pdev, 0);
if (irq <= 0)
--
2.7.4
[PATCH v3 2/2] media: dt-bindings: media: st,stm32-dcmi: Add support of BT656
Add support of BT656 parallel bus mode in DCMI. This mode is enabled when hsync-active & vsync-active fields are not specified. Signed-off-by: Hugues Fruchet --- .../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 ++ 1 file changed, 30 insertions(+) diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml index 3fe778c..1ee521a 100644 --- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml +++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml @@ -44,6 +44,36 @@ properties: bindings defined in Documentation/devicetree/bindings/media/video-interfaces.txt. +properties: + endpoint: +type: object + +properties: + bus-width: true + + hsync-active: +description: + If both HSYNC and VSYNC polarities are not specified, BT656 + embedded synchronization is selected. +default: 0 + + vsync-active: +description: + If both HSYNC and VSYNC polarities are not specified, BT656 + embedded synchronization is selected. +default: 0 + + pclk-sample: true + + remote-endpoint: true + +required: + - remote-endpoint + +additionalProperties: false + +additionalProperties: false + required: - compatible - reg -- 2.7.4
[PATCH v3 0/2] DCMI BT656 parallel bus mode support
Add support of BT656 embedded synchronization bus. This mode allows to save hardware synchro lines hsync & vsync by replacing them with synchro codes embedded in data stream. This mode is enabled when hsync-active & vsync-active fields are not specified in devicetree. === = history = === version 3: - Fix bus_width print to %u as per Sakari comment version 2: - As per Sakari remark, revisit commit message and document BT656 parallel bus mode in bindings version 1: - Initial submission Hugues Fruchet (2): media: stm32-dcmi: add support of BT656 bus media: dt-bindings: media: st,stm32-dcmi: Add support of BT656 .../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 ++ drivers/media/platform/stm32/stm32-dcmi.c | 37 -- 2 files changed, 65 insertions(+), 2 deletions(-) -- 2.7.4
[PATCH v2 1/2] media: stm32-dcmi: add support of BT656 bus
Add support of BT656 embedded synchronization busa
Add support of BT656 embedded synchronization bus.
This mode allows to save hardware synchro lines hsync & vsync by replacing
them with synchro codes embedded in data stream.
This bus type is only compatible with 8 bits width data bus.
Due to reserved values 0x00 & 0xff used for synchro codes, valid data vary
from 0x1 to 0xfe, this is up to sensor to clip accordingly pixel data. As
a consequence of this clipping, JPEG is not supported with this bus type.
DCMI crop feature is also not available with this bus type.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 37 +--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index fd1c41c..d7d7cdb 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -157,6 +157,7 @@ struct stm32_dcmi {
struct vb2_queuequeue;
struct v4l2_fwnode_bus_parallel bus;
+ enum v4l2_mbus_type bus_type;
struct completion complete;
struct clk *mclk;
enum state state;
@@ -777,6 +778,23 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
if (dcmi->bus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
val |= CR_PCKPOL;
+ /*
+* BT656 embedded synchronisation bus mode.
+*
+* Default SAV/EAV mode is supported here with default codes
+* SAV=0xff80 & EAV=0xff9d.
+* With DCMI this means LSC=SAV=0x80 & LEC=EAV=0x9d.
+*/
+ if (dcmi->bus_type == V4L2_MBUS_BT656) {
+ val |= CR_ESS;
+
+ /* Unmask all codes */
+ reg_write(dcmi->regs, DCMI_ESUR, 0x);/* FEC:LEC:LSC:FSC
*/
+
+ /* Trig on LSC=0x80 & LEC=0x9d codes, ignore FSC and FEC */
+ reg_write(dcmi->regs, DCMI_ESCR, 0xff9d80ff);/* FEC:LEC:LSC:FSC
*/
+ }
+
reg_write(dcmi->regs, DCMI_CR, val);
/* Set crop */
@@ -1067,8 +1085,9 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct
v4l2_format *f)
if (ret)
return ret;
- /* Disable crop if JPEG is requested */
- if (pix->pixelformat == V4L2_PIX_FMT_JPEG)
+ /* Disable crop if JPEG is requested or BT656 bus is selected */
+ if (pix->pixelformat == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type != V4L2_MBUS_BT656)
dcmi->do_crop = false;
/* pix to mbus format */
@@ -1592,6 +1611,11 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
if (dcmi_formats[i].mbus_code != mbus_code.code)
continue;
+ /* Exclude JPEG if BT656 bus is selected */
+ if (dcmi_formats[i].fourcc == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type == V4L2_MBUS_BT656)
+ continue;
+
/* Code supported, have we got this fourcc yet? */
for (j = 0; j < num_fmts; j++)
if (sd_fmts[j]->fourcc ==
@@ -1873,9 +1897,18 @@ static int dcmi_probe(struct platform_device *pdev)
dev_err(>dev, "CSI bus not supported\n");
return -ENODEV;
}
+
+ if (ep.bus_type == V4L2_MBUS_BT656 &&
+ ep.bus.parallel.bus_width != 8) {
+ dev_err(>dev, "BT656 bus conflicts with %d bits bus width
(8 bits required)\n",
+ ep.bus.parallel.bus_width);
+ return -ENODEV;
+ }
+
dcmi->bus.flags = ep.bus.parallel.flags;
dcmi->bus.bus_width = ep.bus.parallel.bus_width;
dcmi->bus.data_shift = ep.bus.parallel.data_shift;
+ dcmi->bus_type = ep.bus_type;
irq = platform_get_irq(pdev, 0);
if (irq <= 0)
--
2.7.4
[PATCH v2 0/2] DCMI BT656 parallel bus mode support
Add support of BT656 embedded synchronization bus. This mode allows to save hardware synchro lines hsync & vsync by replacing them with synchro codes embedded in data stream. This mode is enabled when hsync-active & vsync-active fields are not specified in devicetree. === = history = === version 2: - As per Sakari remark, revisit commit message and document BT656 parallel bus mode in bindings version 1: - Initial submission Hugues Fruchet (2): media: stm32-dcmi: add support of BT656 bus media: dt-bindings: media: st,stm32-dcmi: Add support of BT656 .../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 ++ drivers/media/platform/stm32/stm32-dcmi.c | 37 -- 2 files changed, 65 insertions(+), 2 deletions(-) -- 2.7.4
[PATCH v2 2/2] media: dt-bindings: media: st,stm32-dcmi: Add support of BT656
Add support of BT656 parallel bus mode in DCMI. This mode is enabled when hsync-active & vsync-active fields are not specified. Signed-off-by: Hugues Fruchet --- .../devicetree/bindings/media/st,stm32-dcmi.yaml | 30 ++ 1 file changed, 30 insertions(+) diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml index 3fe778c..1ee521a 100644 --- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml +++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml @@ -44,6 +44,36 @@ properties: bindings defined in Documentation/devicetree/bindings/media/video-interfaces.txt. +properties: + endpoint: +type: object + +properties: + bus-width: true + + hsync-active: +description: + If both HSYNC and VSYNC polarities are not specified, BT656 + embedded synchronization is selected. +default: 0 + + vsync-active: +description: + If both HSYNC and VSYNC polarities are not specified, BT656 + embedded synchronization is selected. +default: 0 + + pclk-sample: true + + remote-endpoint: true + +required: + - remote-endpoint + +additionalProperties: false + +additionalProperties: false + required: - compatible - reg -- 2.7.4
Re: [PATCH] media: stm32-dcmi: add support of BT656 bus
Hi Sakari,
I have questions about "bus-type" handling below.
On 10/15/20 5:41 PM, Hugues FRUCHET wrote:
> Hi Sakari,
>
> Thanks for reviewing,
>
> On 10/13/20 11:07 AM, Sakari Ailus wrote:
>> Hi Hugues,
>>
>> On Wed, Oct 07, 2020 at 06:14:50PM +0200, Hugues Fruchet wrote:
>>> Add support of BT656 embedded synchronization bus.
>>> This mode allows to save hardware synchro lines hsync & vsync
>>> by replacing them with synchro codes embedded in data stream.
>>> This bus type is only compatible with 8 bits width data bus.
>>> Due to reserved values 0x00 & 0xff used for synchro codes,
>>> valid data only vary from 0x1 to 0xfe, this is up to sensor
>>> to clip accordingly pixel data. As a consequence of this
>>> clipping, JPEG is not supported when using this bus type.
>>> DCMI crop feature is also not available with this bus type.
>>
>> You can have more than 62 characters per line. In fact, 75 is the
>> recommended maximum.
>>
>> You should also amend the bindings to cover BT.656 mode. Also bus-type
>> should probably be made mandatory, too.
> Will do both.
>
My understanding was that parallel BT656 bus is handled by the absence
of hsync/vsync-active, as stated in
Documentation/devicetree/bindings/media/video-interfaces.txt:
" Note, that if HSYNC and VSYNC polarities are not specified, embedded
synchronization may be required, where supported. "
Do you want to enforce usage of "bus-type" now in order to be more
explicit on parallel or bt656 ?
If I change binding to make "bus-type" required, I have to change the
current board devicetree files to add support of it, and I would prefer
to not do that. Is there a way to put "bus-type" as not mandatory and
rely on absence of hsync/vysnc to trig BT656 ? I will nevertheless amend
bindings in order to document that.
What do you suggest ?
>>
>>>
>>> Signed-off-by: Hugues Fruchet
>>> ---
>>> drivers/media/platform/stm32/stm32-dcmi.c | 37
>>> +--
>>> 1 file changed, 35 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
>>> b/drivers/media/platform/stm32/stm32-dcmi.c
>>> index fd1c41c..d7d7cdb 100644
>>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>>> @@ -157,6 +157,7 @@ struct stm32_dcmi {
>>> struct vb2_queue queue;
>>> struct v4l2_fwnode_bus_parallel bus;
>>> + enum v4l2_mbus_type bus_type;
>>> struct completion complete;
>>> struct clk *mclk;
>>> enum state state;
>>> @@ -777,6 +778,23 @@ static int dcmi_start_streaming(struct vb2_queue
>>> *vq, unsigned int count)
>>> if (dcmi->bus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
>>> val |= CR_PCKPOL;
>>> + /*
>>> + * BT656 embedded synchronisation bus mode.
>>> + *
>>> + * Default SAV/EAV mode is supported here with default codes
>>> + * SAV=0xff80 & EAV=0xff9d.
>>> + * With DCMI this means LSC=SAV=0x80 & LEC=EAV=0x9d.
>>> + */
>>> + if (dcmi->bus_type == V4L2_MBUS_BT656) {
>>> + val |= CR_ESS;
>>> +
>>> + /* Unmask all codes */
>>> + reg_write(dcmi->regs, DCMI_ESUR, 0x);/*
>>> FEC:LEC:LSC:FSC */
>>> +
>>> + /* Trig on LSC=0x80 & LEC=0x9d codes, ignore FSC and FEC */
>>> + reg_write(dcmi->regs, DCMI_ESCR, 0xff9d80ff);/*
>>> FEC:LEC:LSC:FSC */
>>> + }
>>> +
>>> reg_write(dcmi->regs, DCMI_CR, val);
>>> /* Set crop */
>>> @@ -1067,8 +1085,9 @@ static int dcmi_set_fmt(struct stm32_dcmi
>>> *dcmi, struct v4l2_format *f)
>>> if (ret)
>>> return ret;
>>> - /* Disable crop if JPEG is requested */
>>> - if (pix->pixelformat == V4L2_PIX_FMT_JPEG)
>>> + /* Disable crop if JPEG is requested or BT656 bus is selected */
>>> + if (pix->pixelformat == V4L2_PIX_FMT_JPEG &&
>>> + dcmi->bus_type != V4L2_MBUS_BT656)
>>> dcmi->do_crop = false;
>>> /* pix to mbus format */
>>> @@ -1592,6 +1611,11 @@ static int dcmi_formats_init(struct stm32_dcmi
>>> *dcmi)
>>> if (dcmi_formats[i].mbus_code != mbus_code.code)
>>> continue;
>>
Re: [PATCH] media: stm32-dcmi: add support of BT656 bus
Hi Sakari,
Thanks for reviewing,
On 10/13/20 11:07 AM, Sakari Ailus wrote:
> Hi Hugues,
>
> On Wed, Oct 07, 2020 at 06:14:50PM +0200, Hugues Fruchet wrote:
>> Add support of BT656 embedded synchronization bus.
>> This mode allows to save hardware synchro lines hsync & vsync
>> by replacing them with synchro codes embedded in data stream.
>> This bus type is only compatible with 8 bits width data bus.
>> Due to reserved values 0x00 & 0xff used for synchro codes,
>> valid data only vary from 0x1 to 0xfe, this is up to sensor
>> to clip accordingly pixel data. As a consequence of this
>> clipping, JPEG is not supported when using this bus type.
>> DCMI crop feature is also not available with this bus type.
>
> You can have more than 62 characters per line. In fact, 75 is the
> recommended maximum.
>
> You should also amend the bindings to cover BT.656 mode. Also bus-type
> should probably be made mandatory, too.
Will do both.
>
>>
>> Signed-off-by: Hugues Fruchet
>> ---
>> drivers/media/platform/stm32/stm32-dcmi.c | 37
>> +--
>> 1 file changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
>> b/drivers/media/platform/stm32/stm32-dcmi.c
>> index fd1c41c..d7d7cdb 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -157,6 +157,7 @@ struct stm32_dcmi {
>> struct vb2_queuequeue;
>>
>> struct v4l2_fwnode_bus_parallel bus;
>> +enum v4l2_mbus_type bus_type;
>> struct completion complete;
>> struct clk *mclk;
>> enum state state;
>> @@ -777,6 +778,23 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
>> unsigned int count)
>> if (dcmi->bus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
>> val |= CR_PCKPOL;
>>
>> +/*
>> + * BT656 embedded synchronisation bus mode.
>> + *
>> + * Default SAV/EAV mode is supported here with default codes
>> + * SAV=0xff80 & EAV=0xff9d.
>> + * With DCMI this means LSC=SAV=0x80 & LEC=EAV=0x9d.
>> + */
>> +if (dcmi->bus_type == V4L2_MBUS_BT656) {
>> +val |= CR_ESS;
>> +
>> +/* Unmask all codes */
>> +reg_write(dcmi->regs, DCMI_ESUR, 0x);/* FEC:LEC:LSC:FSC
>> */
>> +
>> +/* Trig on LSC=0x80 & LEC=0x9d codes, ignore FSC and FEC */
>> +reg_write(dcmi->regs, DCMI_ESCR, 0xff9d80ff);/* FEC:LEC:LSC:FSC
>> */
>> +}
>> +
>> reg_write(dcmi->regs, DCMI_CR, val);
>>
>> /* Set crop */
>> @@ -1067,8 +1085,9 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi,
>> struct v4l2_format *f)
>> if (ret)
>> return ret;
>>
>> -/* Disable crop if JPEG is requested */
>> -if (pix->pixelformat == V4L2_PIX_FMT_JPEG)
>> +/* Disable crop if JPEG is requested or BT656 bus is selected */
>> +if (pix->pixelformat == V4L2_PIX_FMT_JPEG &&
>> +dcmi->bus_type != V4L2_MBUS_BT656)
>> dcmi->do_crop = false;
>>
>> /* pix to mbus format */
>> @@ -1592,6 +1611,11 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
>> if (dcmi_formats[i].mbus_code != mbus_code.code)
>> continue;
>>
>> +/* Exclude JPEG if BT656 bus is selected */
>> +if (dcmi_formats[i].fourcc == V4L2_PIX_FMT_JPEG &&
>> +dcmi->bus_type == V4L2_MBUS_BT656)
>> +continue;
>> +
>> /* Code supported, have we got this fourcc yet? */
>> for (j = 0; j < num_fmts; j++)
>> if (sd_fmts[j]->fourcc ==
>> @@ -1873,9 +1897,18 @@ static int dcmi_probe(struct platform_device *pdev)
>> dev_err(>dev, "CSI bus not supported\n");
>> return -ENODEV;
>> }
>> +
>> +if (ep.bus_type == V4L2_MBUS_BT656 &&
>> +ep.bus.parallel.bus_width != 8) {
>> +dev_err(>dev, "BT656 bus conflicts with %d bits bus width
>> (8 bits required)\n",
>> +ep.bus.parallel.bus_width);
>
> bus_width is unsigned here.
I will fix it.
>
>> +return -ENODEV;
>> +}
>> +
>> dcmi->bus.flags = ep.bus.parallel.flags;
>> dcmi->bus.bus_width = ep.bus.parallel.bus_width;
>> dcmi->bus.data_shift = ep.bus.parallel.data_shift;
>> +dcmi->bus_type = ep.bus_type;
>>
>> irq = platform_get_irq(pdev, 0);
>> if (irq <= 0)
>
BR,
Hugues.
[PATCH] ARM: dts: stm32: fix DCMI DMA features on stm32mp15 family
Enable FIFO mode with half-full threshold. Signed-off-by: Hugues Fruchet --- arch/arm/boot/dts/stm32mp151.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi index bfe2902..cfba9a1 100644 --- a/arch/arm/boot/dts/stm32mp151.dtsi +++ b/arch/arm/boot/dts/stm32mp151.dtsi @@ -1091,7 +1091,7 @@ resets = < CAMITF_R>; clocks = < DCMI>; clock-names = "mclk"; - dmas = < 75 0x400 0x0d>; + dmas = < 75 0x400 0x01>; dma-names = "tx"; status = "disabled"; }; -- 2.7.4
[PATCH] media: stm32-dcmi: add support of BT656 bus
Add support of BT656 embedded synchronization bus.
This mode allows to save hardware synchro lines hsync & vsync
by replacing them with synchro codes embedded in data stream.
This bus type is only compatible with 8 bits width data bus.
Due to reserved values 0x00 & 0xff used for synchro codes,
valid data only vary from 0x1 to 0xfe, this is up to sensor
to clip accordingly pixel data. As a consequence of this
clipping, JPEG is not supported when using this bus type.
DCMI crop feature is also not available with this bus type.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 37 +--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index fd1c41c..d7d7cdb 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -157,6 +157,7 @@ struct stm32_dcmi {
struct vb2_queuequeue;
struct v4l2_fwnode_bus_parallel bus;
+ enum v4l2_mbus_type bus_type;
struct completion complete;
struct clk *mclk;
enum state state;
@@ -777,6 +778,23 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
if (dcmi->bus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
val |= CR_PCKPOL;
+ /*
+* BT656 embedded synchronisation bus mode.
+*
+* Default SAV/EAV mode is supported here with default codes
+* SAV=0xff80 & EAV=0xff9d.
+* With DCMI this means LSC=SAV=0x80 & LEC=EAV=0x9d.
+*/
+ if (dcmi->bus_type == V4L2_MBUS_BT656) {
+ val |= CR_ESS;
+
+ /* Unmask all codes */
+ reg_write(dcmi->regs, DCMI_ESUR, 0x);/* FEC:LEC:LSC:FSC
*/
+
+ /* Trig on LSC=0x80 & LEC=0x9d codes, ignore FSC and FEC */
+ reg_write(dcmi->regs, DCMI_ESCR, 0xff9d80ff);/* FEC:LEC:LSC:FSC
*/
+ }
+
reg_write(dcmi->regs, DCMI_CR, val);
/* Set crop */
@@ -1067,8 +1085,9 @@ static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct
v4l2_format *f)
if (ret)
return ret;
- /* Disable crop if JPEG is requested */
- if (pix->pixelformat == V4L2_PIX_FMT_JPEG)
+ /* Disable crop if JPEG is requested or BT656 bus is selected */
+ if (pix->pixelformat == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type != V4L2_MBUS_BT656)
dcmi->do_crop = false;
/* pix to mbus format */
@@ -1592,6 +1611,11 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
if (dcmi_formats[i].mbus_code != mbus_code.code)
continue;
+ /* Exclude JPEG if BT656 bus is selected */
+ if (dcmi_formats[i].fourcc == V4L2_PIX_FMT_JPEG &&
+ dcmi->bus_type == V4L2_MBUS_BT656)
+ continue;
+
/* Code supported, have we got this fourcc yet? */
for (j = 0; j < num_fmts; j++)
if (sd_fmts[j]->fourcc ==
@@ -1873,9 +1897,18 @@ static int dcmi_probe(struct platform_device *pdev)
dev_err(>dev, "CSI bus not supported\n");
return -ENODEV;
}
+
+ if (ep.bus_type == V4L2_MBUS_BT656 &&
+ ep.bus.parallel.bus_width != 8) {
+ dev_err(>dev, "BT656 bus conflicts with %d bits bus width
(8 bits required)\n",
+ ep.bus.parallel.bus_width);
+ return -ENODEV;
+ }
+
dcmi->bus.flags = ep.bus.parallel.flags;
dcmi->bus.bus_width = ep.bus.parallel.bus_width;
dcmi->bus.data_shift = ep.bus.parallel.data_shift;
+ dcmi->bus_type = ep.bus_type;
irq = platform_get_irq(pdev, 0);
if (irq <= 0)
--
2.7.4
[PATCH] media: stm32-dcmi: add 8-bit Bayer formats support
From: Alain Volmat
Add BA81, GBRG, GRBG, RGGB formats in the list of
supported capture formats.
Signed-off-by: Alain Volmat
Acked-by: Hugues FRUCHET
Reviewed-by: Philippe CORNU
---
drivers/media/platform/stm32/stm32-dcmi.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index fd1c41c..918b41d 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1574,6 +1574,22 @@ static const struct dcmi_format dcmi_formats[] = {
.fourcc = V4L2_PIX_FMT_JPEG,
.mbus_code = MEDIA_BUS_FMT_JPEG_1X8,
.bpp = 1,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SBGGR8,
+ .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8,
+ .bpp = 1,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SGBRG8,
+ .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8,
+ .bpp = 1,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SGRBG8,
+ .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8,
+ .bpp = 1,
+ }, {
+ .fourcc = V4L2_PIX_FMT_SRGGB8,
+ .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8,
+ .bpp = 1,
},
};
--
2.7.4
[PATCH] media: stm32-dcmi: remove deprecated dmaengine_terminate_all()
Replace dmaengine_terminate_all() by dmaengine_terminate_sync() to ensure that all pending dma operations are really finished. This is not ensured by dmaengine_terminate_all() and this API is deprecated, so better to use the _sync() variant. Signed-off-by: Hugues Fruchet Reviewed-by: Philippe CORNU --- drivers/media/platform/stm32/stm32-dcmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c index fd1c41c..3283337 100644 --- a/drivers/media/platform/stm32/stm32-dcmi.c +++ b/drivers/media/platform/stm32/stm32-dcmi.c @@ -324,7 +324,7 @@ static int dcmi_start_dma(struct stm32_dcmi *dcmi, } /* -* Avoid call of dmaengine_terminate_all() between +* Avoid call of dmaengine_terminate_sync() between * dmaengine_prep_slave_single() and dmaengine_submit() * by locking the whole DMA submission sequence */ @@ -438,7 +438,7 @@ static void dcmi_process_jpeg(struct stm32_dcmi *dcmi) } /* Abort DMA operation */ - dmaengine_terminate_all(dcmi->dma_chan); + dmaengine_terminate_sync(dcmi->dma_chan); /* Restart capture */ if (dcmi_restart_capture(dcmi)) @@ -882,7 +882,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq) /* Stop all pending DMA operations */ mutex_lock(>dma_lock); - dmaengine_terminate_all(dcmi->dma_chan); + dmaengine_terminate_sync(dcmi->dma_chan); mutex_unlock(>dma_lock); pm_runtime_put(dcmi->dev); -- 2.7.4
[PATCH] media: stm32-dcmi: don't print an error on probe deferral
From: Etienne Carriere
Change stm32-dcmi driver to not print an error message when the
device probe operation is deferred.
Signed-off-by: Etienne Carriere
Acked-by: Hugues Fruchet
Tested-by: Alexandre TORGUE
---
drivers/media/platform/stm32/stm32-dcmi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index fd1c41c..720534e 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1851,7 +1851,9 @@ static int dcmi_probe(struct platform_device *pdev)
dcmi->rstc = devm_reset_control_get_exclusive(>dev, NULL);
if (IS_ERR(dcmi->rstc)) {
- dev_err(>dev, "Could not get reset control\n");
+ if (PTR_ERR(dcmi->rstc) != -EPROBE_DEFER)
+ dev_err(>dev, "Could not get reset control\n");
+
return PTR_ERR(dcmi->rstc);
}
--
2.7.4
Re: [PATCH v2 3/4] media: i2c: ov5640: Add support for BT656 mode
Hi Prabhakar,
I'm currently testing the OV5640 CCIR656 embedded synchronisation mode
on STM32MP1 running STM32 DCMI camera interface.
Tests not yet fully completed but sounds good, more details below.
On 8/3/20 4:31 PM, Lad Prabhakar wrote:
> Enable support for BT656 mode.
>
> Signed-off-by: Lad Prabhakar
> Reviewed-by: Biju Das
> ---
> drivers/media/i2c/ov5640.c | 40 +++---
> 1 file changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index ec444bee2ce9..08c67250042f 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -82,6 +82,7 @@
> #define OV5640_REG_VFIFO_HSIZE 0x4602
> #define OV5640_REG_VFIFO_VSIZE 0x4604
> #define OV5640_REG_JPG_MODE_SELECT 0x4713
> +#define OV5640_REG_CCIR656_CTRL000x4730
> #define OV5640_REG_POLARITY_CTRL00 0x4740
> #define OV5640_REG_MIPI_CTRL00 0x4800
> #define OV5640_REG_DEBUG_MODE 0x4814
> @@ -1216,6 +1217,18 @@ static int ov5640_set_autogain(struct ov5640_dev
> *sensor, bool on)
> BIT(1), on ? 0 : BIT(1));
> }
>
> +static int ov5640_set_stream_bt656(struct ov5640_dev *sensor, bool on)
> +{
> + int ret;
> +
> + ret = ov5640_write_reg(sensor, OV5640_REG_CCIR656_CTRL00, on ? 0x1 :
> 0x00);
> + if (ret)
> + return ret;
Please add a comment explaining bit fields from datasheet:
Bit[7]: SYNC code selection
0: Automatically generate SYNC code
1: SYNC code from register setting 0x4732~4735
Bit[4:3]: Blank toggle data options
00: Toggle data is 1'h040/1'h200
Bit[1]: Clip data disable (so clip is enabled)
Bit[0]: CCIR656 mode enable
So 0x1 stands for CCIR656 with automatic SYNC codes: SAV/EAV with
default values ie SAV=0xFF80 & EAV=0xFF9d.
On STM32 platform, this correspond to DCMI configuration ESCR=0xff9d80ff
and ESUR=0x.
On Renesas platform, I have not seen any configuration in code, this
SAV/EAV mode seems to be handled by default by hardware, do you confirm ?
Note that another CCIR656 embedded synchro mode could be used with
custom synchro codes FS/FE/LS/LE in registers 0x4732-0x4735, this
mode is enabled with CCIR656_CTRL00(0x4730) set to 0x81:
Bit[7]: SYNC code selection
1: SYNC code from register setting 0x4732~4735
> +
> + return ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ?
> + OV5640_SOFTWARE_WAKEUP : OV5640_SOFTWARE_PWDN);
> +}
> +
> static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
> {
> return ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ?
> @@ -2022,18 +2035,20 @@ static int ov5640_set_dvp(struct ov5640_dev *sensor,
> bool on)
>* datasheet and hardware, 0 is active high
>* and 1 is active low...)
>*/
> - if (flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
> - pclk_pol = 1;
> - if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> - hsync_pol = 1;
> - if (flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
> - vsync_pol = 1;
> + if (sensor->ep.bus_type == V4L2_MBUS_PARALLEL) {
> + if (flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
> + pclk_pol = 1;
> + if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> + hsync_pol = 1;
> + if (flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
> + vsync_pol = 1;
>
> - ret = ov5640_write_reg(sensor, OV5640_REG_POLARITY_CTRL00,
> -(pclk_pol << 5) | (hsync_pol << 1) | vsync_pol);
> + ret = ov5640_write_reg(sensor, OV5640_REG_POLARITY_CTRL00,
> +(pclk_pol << 5) | (hsync_pol << 1) |
> vsync_pol);
>
> - if (ret)
> - return ret;
> + if (ret)
> + return ret;
> + }
>
> /*
>* powerdown MIPI TX/RX PHY & disable MIPI
> @@ -2057,7 +2072,8 @@ static int ov5640_set_dvp(struct ov5640_dev *sensor,
> bool on)
>* - 4: PCLK output enable
>* - [3:0]: D[9:6] output enable
>*/
> - ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE01, 0x7f);
> + ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE01,
> +sensor->ep.bus_type == V4L2_MBUS_PARALLEL ? 0x7f
> : 0x1f);
> if (ret)
> return ret;
>
> @@ -2911,6 +2927,8 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int
> enable)
>
> if (sensor->ep.bus_type == V4L2_MBUS_CSI2_DPHY)
> ret = ov5640_set_stream_mipi(sensor, enable);
> + else if (sensor->ep.bus_type == V4L2_MBUS_BT656)
> + ret = ov5640_set_stream_bt656(sensor, enable);
> else
> ret = ov5640_set_stream_dvp(sensor, enable);
>
>
BR, Hugues.
Re: [PATCH v4 1/6] media: i2c: ov5640: Remain in power down for DVP mode unless streaming
Hi Prabhakar, As discussed separately I would prefer to better understand issue before going to this patch. Nevertheless I have some remarks in code in case we'll need it at the end. On 9/4/20 10:18 PM, Lad Prabhakar wrote: > Keep the sensor in software power down mode and wake up only in > ov5640_set_stream_dvp() callback. > > Signed-off-by: Lad Prabhakar > Reviewed-by: Biju Das > Tested-by: Jacopo Mondi > --- > drivers/media/i2c/ov5640.c | 19 --- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > index 2fe4a7ac0592..880fde73a030 100644 > --- a/drivers/media/i2c/ov5640.c > +++ b/drivers/media/i2c/ov5640.c > @@ -34,6 +34,8 @@ > #define OV5640_REG_SYS_RESET02 0x3002 > #define OV5640_REG_SYS_CLOCK_ENABLE02 0x3006 > #define OV5640_REG_SYS_CTRL00x3008 > +#define OV5640_REG_SYS_CTRL0_SW_PWDN 0x42 > +#define OV5640_REG_SYS_CTRL0_SW_PWUP 0x02 For the time being this section was only referring to registers addresses and bit details was explained into a comment right before affectation, see OV5640_REG_IO_MIPI_CTRL00 for example. > #define OV5640_REG_CHIP_ID 0x300a > #define OV5640_REG_IO_MIPI_CTRL00 0x300e > #define OV5640_REG_PAD_OUTPUT_ENABLE01 0x3017 > @@ -1120,6 +1122,12 @@ static int ov5640_load_regs(struct ov5640_dev *sensor, > val = regs->val; > mask = regs->mask; > > + /* remain in power down mode for DVP */ > + if (regs->reg_addr == OV5640_REG_SYS_CTRL0 && > + val == OV5640_REG_SYS_CTRL0_SW_PWUP && > + sensor->ep.bus_type != V4L2_MBUS_CSI2_DPHY) > + continue; > + I understand that more or less register OV5640_REG_SYS_CTRL0 (0x3008) has been partially removed from big init sequence: for power-up part, but power-dwn remains at very beginning of sequence. We should completely remove 0x3008 from init sequence, including power-dwn, and introduce a new function ov5640_sw_powerdown(on/off) that should be called at the right place instead. > if (mask) > ret = ov5640_mod_reg(sensor, reg_addr, mask, val); > else > @@ -1297,9 +1305,14 @@ static int ov5640_set_stream_dvp(struct ov5640_dev > *sensor, bool on) >* PAD OUTPUT ENABLE 02 >* - [7:2]: D[5:0] output enable >*/ > - return ov5640_write_reg(sensor, > - OV5640_REG_PAD_OUTPUT_ENABLE02, > - on ? 0xfc : 0); > + ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE02, > +on ? 0xfc : 0); > + if (ret) > + return ret; > + > + return ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ? > + OV5640_REG_SYS_CTRL0_SW_PWUP : > + OV5640_REG_SYS_CTRL0_SW_PWDN); > } > > static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on) > BR, Hugues.
Re: [PATCH v4 3/6] media: i2c: ov5640: Enable data pins on poweron for DVP mode
Hi Prabhakar, I have checked quickly both OK and KO traces an some differences are ther that we have to understand, see bottom of this mail. On 9/7/20 4:35 PM, Lad, Prabhakar wrote: > Hi Hugues, > > Thank you for the review. > > On Mon, Sep 7, 2020 at 10:44 AM Hugues FRUCHET wrote: >> >> Hi Prabhakar, >> >> Thanks for your patches, good to see one more OV5640 stakeholder >> upstreaming some fixes/features. >> >> I'm also using a parallel setup with OV5640 connected on STM32 DCMI >> camera interface. >> First basic tests have not shown any regressions on my side but I would >> like to better understand the problem you encountered and the way you >> solve it, see below my comments. >> >> > Thank you for testing the patches. > >> On 9/4/20 10:18 PM, Lad Prabhakar wrote: >>> During testing this sensor on iW-RainboW-G21D-Qseven platform in 8-bit DVP >>> mode with rcar-vin bridge noticed the capture worked fine for the first run >>> (with yavta), but for subsequent runs the bridge driver waited for the >>> frame to be captured. Debugging further noticed the data lines were >>> enabled/disabled in stream on/off callback and dumping the register >>> contents 0x3017/0x3018 in ov5640_set_stream_dvp() reported the correct >>> values, but yet frame capturing failed. >> >> Could you show the sequence of V4L2 calls which lead to freeze ? >> >> Reading the patch you proposed, my guess is that issue is coming when >> multiple S_STREAM(on)/S_STREAM(off) are made while power remains, is >> that true ? So reading your traces, we are not in that case... >> I have added some traces in code and tried to reproduce with yavta, >> v4l2-ctl and GStreamer but I'm not able to generate such sequence, here >> is what I got everytime: >> >> [ 809.113790] ov5640 0-003c: ov5640_s_power> >> [ 809.116431] ov5640 0-003c: ov5640_set_power> >> [ 809.120788] ov5640 0-003c: ov5640_set_power_on> >> [ 809.622047] ov5640 0-003c: ov5640_set_power_dvp> >> [ 809.862734] ov5640 0-003c: ov5640_s_stream> >> [ 809.865462] ov5640 0-003c: ov5640_set_stream_dvp on> >> >> [ 828.549531] ov5640 0-003c: ov5640_s_stream> >> [ 828.552265] ov5640 0-003c: ov5640_set_stream_dvp off> >> [ 828.580970] ov5640 0-003c: ov5640_s_power> >> [ 828.583613] ov5640 0-003c: ov5640_set_power> >> [ 828.587921] ov5640 0-003c: ov5640_set_power_dvp> >> [ 828.620346] ov5640 0-003c: ov5640_set_power_off> >> >> Which application/command line are you using to reproduce your problem ? >> > yavta. >> >>> >>> To get around this issue data lines are enabled in s_power callback. >>> (Also the sensor remains in power down mode if not streaming so power >>> consumption shouldn't be affected) >> >> For the time being, I really don't understand why this patch is fixing >> capture freeze. >> > > Below is the log with this series applied in DVP mode: > > root@iwg21m:~# > root@iwg21m:~# ./yavta /dev/video0 -c1 -n3 -s640x480 -fUYVY -Fov.raw > [ 36.191661] ov5640_s_power> > [ 36.194452] ov5640_set_power> > [ 36.197413] ov5640_set_power_on> > [ 36.200714] ov5640_reset> > [ 36.203328] ov5640_restore_mode> > [ 36.206549] ov5640_load_regs> > [ 36.550255] ov5640_set_timings> > [ 36.554572] ov5640_set_mode> > [ 36.557963] ov5640_calc_pixel_rate> > [ 36.561458] ov5640_set_dvp_pclk> > [ 36.564679] ov5640_calc_pclk> > [ 36.567639] ov5640_calc_sys_clk> > [ 36.572190] ov5640_set_mode_direct> > [ 36.575671] ov5640_load_regs> > [ 36.583205] ov5640_set_timings> > [ 36.591494] ov5640_set_framefmt> > [ 36.595717] ov5640_set_power_dvp> > [ 36.599486] ov5640_s_ctrl> > [ 36.602200] ov5640_set_ctrl_white_balance> > [ 36.606550] ov5640_s_ctrl> > [ 36.609250] ov5640_set_ctrl_exposure> > [ 36.613179] ov5640_s_ctrl> > [ 36.615878] ov5640_set_ctrl_gain> > [ 36.619446] ov5640_s_ctrl> > [ 36.622160] ov5640_set_ctrl_saturation> > [ 36.626476] ov5640_s_ctrl> > [ 36.629177] ov5640_set_ctrl_hue> > [ 36.632670] ov5640_s_ctrl> > [ 36.635370] ov5640_set_ctrl_contrast> > [ 36.639282] ov5640_s_ctrl> > [ 36.642112] ov5640_s_ctrl> > [ 36.644813] ov5640_set_ctrl_hflip> > [ 36.648465] ov5640_s_ctrl> > [ 36.651179] ov5640_set_ctrl_vflip> > [ 36.654833] ov5640_s_ctrl> > [ 36.657533] ov5640_set_ctrl_light_freq> > Device /dev/video0 opened. > Device `R_Car_VIN' on `platform:e6ef3[ 36.662120] o
Re: [PATCH v4 3/6] media: i2c: ov5640: Enable data pins on poweron for DVP mode
Hi Prabhakar,
Thanks for your patches, good to see one more OV5640 stakeholder
upstreaming some fixes/features.
I'm also using a parallel setup with OV5640 connected on STM32 DCMI
camera interface.
First basic tests have not shown any regressions on my side but I would
like to better understand the problem you encountered and the way you
solve it, see below my comments.
On 9/4/20 10:18 PM, Lad Prabhakar wrote:
> During testing this sensor on iW-RainboW-G21D-Qseven platform in 8-bit DVP
> mode with rcar-vin bridge noticed the capture worked fine for the first run
> (with yavta), but for subsequent runs the bridge driver waited for the
> frame to be captured. Debugging further noticed the data lines were
> enabled/disabled in stream on/off callback and dumping the register
> contents 0x3017/0x3018 in ov5640_set_stream_dvp() reported the correct
> values, but yet frame capturing failed.
Could you show the sequence of V4L2 calls which lead to freeze ?
Reading the patch you proposed, my guess is that issue is coming when
multiple S_STREAM(on)/S_STREAM(off) are made while power remains, is
that true ?
I have added some traces in code and tried to reproduce with yavta,
v4l2-ctl and GStreamer but I'm not able to generate such sequence, here
is what I got everytime:
[ 809.113790] ov5640 0-003c: ov5640_s_power>
[ 809.116431] ov5640 0-003c: ov5640_set_power>
[ 809.120788] ov5640 0-003c: ov5640_set_power_on>
[ 809.622047] ov5640 0-003c: ov5640_set_power_dvp>
[ 809.862734] ov5640 0-003c: ov5640_s_stream>
[ 809.865462] ov5640 0-003c: ov5640_set_stream_dvp on>
[ 828.549531] ov5640 0-003c: ov5640_s_stream>
[ 828.552265] ov5640 0-003c: ov5640_set_stream_dvp off>
[ 828.580970] ov5640 0-003c: ov5640_s_power>
[ 828.583613] ov5640 0-003c: ov5640_set_power>
[ 828.587921] ov5640 0-003c: ov5640_set_power_dvp>
[ 828.620346] ov5640 0-003c: ov5640_set_power_off>
Which application/command line are you using to reproduce your problem ?
>
> To get around this issue data lines are enabled in s_power callback.
> (Also the sensor remains in power down mode if not streaming so power
> consumption shouldn't be affected)
For the time being, I really don't understand why this patch is fixing
capture freeze.
>
> Fixes: f22996db44e2d ("media: ov5640: add support of DVP parallel interface")
> Signed-off-by: Lad Prabhakar
> Reviewed-by: Biju Das
> Tested-by: Jacopo Mondi
> ---
> drivers/media/i2c/ov5640.c | 73 +-
> 1 file changed, 40 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 8af11d532699..8288728d8704 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -276,8 +276,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct
> v4l2_ctrl *ctrl)
> /* YUV422 UYVY VGA@30fps */
> static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
> {0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
> - {0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
> - {0x3630, 0x36, 0, 0},
> + {0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0},
> {0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0},
> {0x3621, 0xe0, 0, 0}, {0x3704, 0xa0, 0, 0}, {0x3703, 0x5a, 0, 0},
> {0x3715, 0x78, 0, 0}, {0x3717, 0x01, 0, 0}, {0x370b, 0x60, 0, 0},
> @@ -1283,33 +1282,6 @@ static int ov5640_set_stream_dvp(struct ov5640_dev
> *sensor, bool on)
> if (ret)
> return ret;
>
> - /*
> - * enable VSYNC/HREF/PCLK DVP control lines
> - * & D[9:6] DVP data lines
> - *
> - * PAD OUTPUT ENABLE 01
> - * - 6: VSYNC output enable
> - * - 5: HREF output enable
> - * - 4: PCLK output enable
> - * - [3:0]: D[9:6] output enable
> - */
> - ret = ov5640_write_reg(sensor,
> -OV5640_REG_PAD_OUTPUT_ENABLE01,
> -on ? 0x7f : 0);
> - if (ret)
> - return ret;
> -
> - /*
> - * enable D[5:0] DVP data lines
> - *
> - * PAD OUTPUT ENABLE 02
> - * - [7:2]: D[5:0] output enable
> - */
> - ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE02,
> -on ? 0xfc : 0);
> - if (ret)
> - return ret;
> -
> return ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0, on ?
> OV5640_REG_SYS_CTRL0_SW_PWUP :
> OV5640_REG_SYS_CTRL0_SW_PWDN);
> @@ -2069,6 +2041,40 @@ static int ov5640_set_power_mipi(struct ov5640_dev
> *sensor, bool on)
> return 0;
> }
>
> +static int ov5640_set_power_dvp(struct ov5640_dev *sensor, bool on)
> +{
> + int ret;
> +
> + if (!on) {
> + /* Reset settings to their default values. */
> + ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT_ENABLE01, 0x00);
> + ov5640_write_reg(sensor,
Re: [PATCH 2/2] media: stm32-dcmi: fix probe error path & module remove
Reviewed-by: Hugues Fruchet
On 7/28/20 8:37 AM, Alain Volmat wrote:
> This commit add missing vb2_queue_release calls with the
> probe error path and module remove.
> Missing v4l2_async_notifier_unregister is also added within
> the probe error path
>
> Fixes: 37404f91ef8b ("[media] stm32-dcmi: STM32 DCMI camera interface driver")
> Signed-off-by: Alain Volmat
> ---
> drivers/media/platform/stm32/stm32-dcmi.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index 5e60d4c6eeeb..57830ee691be 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -2004,7 +2004,7 @@ static int dcmi_probe(struct platform_device *pdev)
>
> ret = dcmi_graph_init(dcmi);
> if (ret < 0)
> - goto err_media_entity_cleanup;
> + goto err_vb2_queue_release;
>
> /* Reset device */
> ret = reset_control_assert(dcmi->rstc);
> @@ -2030,7 +2030,10 @@ static int dcmi_probe(struct platform_device *pdev)
> return 0;
>
> err_cleanup:
> + v4l2_async_notifier_unregister(>notifier);
> v4l2_async_notifier_cleanup(>notifier);
> +err_vb2_queue_release:
> + vb2_queue_release(q);
> err_media_entity_cleanup:
> media_entity_cleanup(>vdev->entity);
> err_device_release:
> @@ -2052,6 +2055,7 @@ static int dcmi_remove(struct platform_device *pdev)
>
> v4l2_async_notifier_unregister(>notifier);
> v4l2_async_notifier_cleanup(>notifier);
> + vb2_queue_release(>queue);
> media_entity_cleanup(>vdev->entity);
> v4l2_device_unregister(>v4l2_dev);
> media_device_cleanup(>mdev);
>
Re: [PATCH 1/2] media: stm32-dcmi: create video dev within notifier bound
Reviewed-by: Hugues Fruchet
On 7/28/20 8:37 AM, Alain Volmat wrote:
> In case of an error during the initialization of the sensor,
> the video device is still available since created at the
> probe of the dcmi driver. Moreover the device wouldn't
> be released even when removing the module since the release
> is performed as part of the notifier unbind callback
> (not called if no sensor is properly initialized).
>
> This patch move the video device creation with the v4l2 notifier
> bound handler in order to avoid having a video device created when
> an error happen during the pipe (dcmi - sensor) initialization.
>
> This also makes the video device creation symmetric with the
> release which is already done within the notifier unbind handler.
>
> Fixes: 37404f91ef8b ("[media] stm32-dcmi: STM32 DCMI camera interface driver")
> Signed-off-by: Alain Volmat
> ---
> drivers/media/platform/stm32/stm32-dcmi.c | 23 ---
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index b8931490b83b..5e60d4c6eeeb 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -1747,6 +1747,15 @@ static int dcmi_graph_notify_bound(struct
> v4l2_async_notifier *notifier,
>
> dev_dbg(dcmi->dev, "Subdev \"%s\" bound\n", subdev->name);
>
> + ret = video_register_device(dcmi->vdev, VFL_TYPE_VIDEO, -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));
> +
> /*
>* Link this sub-device to DCMI, it could be
>* a parallel camera sensor or a bridge
> @@ -1759,10 +1768,11 @@ static int dcmi_graph_notify_bound(struct
> v4l2_async_notifier *notifier,
> >vdev->entity, 0,
> MEDIA_LNK_FL_IMMUTABLE |
> MEDIA_LNK_FL_ENABLED);
> - if (ret)
> + if (ret) {
> dev_err(dcmi->dev, "Failed to create media pad link with subdev
> \"%s\"\n",
> subdev->name);
> - else
> + video_unregister_device(dcmi->vdev);
> + } else
> dev_dbg(dcmi->dev, "DCMI is now linked to \"%s\"\n",
> subdev->name);
>
> @@ -1974,15 +1984,6 @@ static int dcmi_probe(struct platform_device *pdev)
> }
> dcmi->vdev->entity.flags |= MEDIA_ENT_FL_DEFAULT;
>
> - ret = video_register_device(dcmi->vdev, VFL_TYPE_VIDEO, -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;
>
Re: [PATCH v6 2/3] media: stm32-dcmi: Set minimum cpufreq requirement
Hi Benjamin,
test condition in set_min_frequency() to fix, appart from that:
Acked-by: Hugues Fruchet
BR,
Hugues.
On 6/10/20 2:24 PM, Benjamin Gaignard wrote:
> Before start streaming set cpufreq minimum frequency requirement.
> The cpufreq governor will adapt the frequencies and we will have
> no latency for handling interrupts.
> The frequency requirement is retrieved from the device-tree node.
>
> Signed-off-by: Benjamin Gaignard
> ---
> version 6:
> - come back to version 4 and follow Valentin's suggestions about notifier
> - add Valentin's comment about notifier set/unset
>
> version 5:
> - add a mutex to protect dcmi_irq_notifier_notify()
> - register notifier a probe time
>
> version 4:
> - simplify irq affinity handling by using only dcmi_irq_notifier_notify()
>
> version 3:
> - add a cpumask field to track boosted CPUs
> - add irq_affinity_notify callback
> - protect cpumask field with a mutex
>
> drivers/media/platform/stm32/stm32-dcmi.c | 138
> --
> 1 file changed, 130 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index b8931490b83b..382df6e7c864 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -13,10 +13,13 @@
>
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -99,6 +102,8 @@ enum state {
>
> #define OVERRUN_ERROR_THRESHOLD 3
>
> +static DEFINE_PER_CPU(struct freq_qos_request, qos_req);
> +
> struct dcmi_graph_entity {
> struct v4l2_async_subdev asd;
>
> @@ -133,6 +138,7 @@ struct stm32_dcmi {
> struct resource *res;
> struct reset_control*rstc;
> int sequence;
> + int irq;
> struct list_headbuffers;
> struct dcmi_buf *active;
>
> @@ -173,6 +179,11 @@ struct stm32_dcmi {
> struct media_device mdev;
> struct media_padvid_cap_pad;
> struct media_pipeline pipeline;
> +
> + struct mutexfreq_lock;
> + u32 min_frequency;
> + cpumask_var_t boosted;
> + struct irq_affinity_notify notify;
> };
>
> static inline struct stm32_dcmi *notifier_to_dcmi(struct
> v4l2_async_notifier *n)
> @@ -722,6 +733,99 @@ static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi)
> dcmi_pipeline_s_stream(dcmi, 0);
> }
>
> +static void dcmi_get_min_frequency(struct stm32_dcmi *dcmi)
> +{
> + struct device_node *np = dcmi->mdev.dev->of_node;
> +
> + dcmi->min_frequency = FREQ_QOS_MIN_DEFAULT_VALUE;
> +
> + of_property_read_u32(np, "st,stm32-dcmi-min-frequency",
> + >min_frequency);
> +}
> +
> +static void dcmi_irq_notifier_notify(struct irq_affinity_notify *notify,
> + const cpumask_t *mask)
> +{
> + struct stm32_dcmi *dcmi = container_of(notify,
> +struct stm32_dcmi,
> +notify);
> + struct cpufreq_policy *p;
> + int cpu;
> +
> + mutex_lock(>freq_lock);
> + /*
> + * For all boosted CPUs check if it is still the case
> + * if not remove the request
> + */
> + for_each_cpu(cpu, dcmi->boosted) {
> + if (cpumask_test_cpu(cpu, mask))
> + continue;
> +
> + p = cpufreq_cpu_get(cpu);
> + if (!p)
> + continue;
> +
> + freq_qos_remove_request(_cpu(qos_req, cpu));
> + cpumask_andnot(dcmi->boosted, dcmi->boosted, p->cpus);
> +
> + cpufreq_cpu_put(p);
> + }
> +
> + /*
> + * For CPUs in the mask check if they are boosted if not add
> + * a request
> + */
> + for_each_cpu(cpu, mask) {
> + if (cpumask_test_cpu(cpu, dcmi->boosted))
> + continue;
> +
> + p = cpufreq_cpu_get(cpu);
> + if (!p)
> + continue;
> +
> + freq_qos_add_request(>constraints, _cpu(qos_req, cpu),
> + FREQ_QOS_MIN, dcmi->min_frequency);
> + cpumask_or(dcmi->boosted, dcmi->boosted, p->cpus);
> + cpufreq_c
Re: [PATCH] media: stm32-dcmi: Set minimum cpufreq requirement
Acked-by: Hugues Fruchet
On 5/27/20 5:16 PM, Benjamin Gaignard wrote:
> Before start streaming set cpufreq minimum frequency requirement.
> The cpufreq governor will adapt the frequencies and we will have
> no latency for handling interrupts.
>
> Signed-off-by: Benjamin Gaignard
> ---
> drivers/media/platform/stm32/stm32-dcmi.c | 29 -
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index b8931490b83b..97c342351569 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -13,6 +13,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -99,6 +100,8 @@ enum state {
>
> #define OVERRUN_ERROR_THRESHOLD 3
>
> +#define DCMI_MIN_FREQ65 /* in KHz */
> +
> struct dcmi_graph_entity {
> struct v4l2_async_subdev asd;
>
> @@ -173,6 +176,10 @@ struct stm32_dcmi {
> struct media_device mdev;
> struct media_padvid_cap_pad;
> struct media_pipeline pipeline;
> +
> + /* CPU freq contraint */
> + struct cpufreq_policy *policy;
> + struct freq_qos_request qos_req;
> };
>
> static inline struct stm32_dcmi *notifier_to_dcmi(struct
> v4l2_async_notifier *n)
> @@ -736,11 +743,20 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
> unsigned int count)
> goto err_release_buffers;
> }
>
> + if (dcmi->policy) {
> + ret = freq_qos_add_request(>policy->constraints,
> +>qos_req, FREQ_QOS_MIN,
> +DCMI_MIN_FREQ);
> +
> + if (ret < 0)
> + goto err_pm_put;
> + }
> +
> ret = media_pipeline_start(>vdev->entity, >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;
> + goto err_drop_qos;
> }
>
> ret = dcmi_pipeline_start(dcmi);
> @@ -835,6 +851,9 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
> unsigned int count)
> err_media_pipeline_stop:
> media_pipeline_stop(>vdev->entity);
>
> +err_drop_qos:
> + if (dcmi->policy)
> + freq_qos_remove_request(>qos_req);
> err_pm_put:
> pm_runtime_put(dcmi->dev);
>
> @@ -863,6 +882,9 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
>
> media_pipeline_stop(>vdev->entity);
>
> + if (dcmi->policy)
> + freq_qos_remove_request(>qos_req);
> +
> spin_lock_irq(>irqlock);
>
> /* Disable interruptions */
> @@ -2020,6 +2042,8 @@ static int dcmi_probe(struct platform_device *pdev)
> goto err_cleanup;
> }
>
> + dcmi->policy = cpufreq_cpu_get(0);
> +
> dev_info(>dev, "Probe done\n");
>
> platform_set_drvdata(pdev, dcmi);
> @@ -2049,6 +2073,9 @@ static int dcmi_remove(struct platform_device *pdev)
>
> pm_runtime_disable(>dev);
>
> + if (dcmi->policy)
> + cpufreq_cpu_put(dcmi->policy);
> +
> v4l2_async_notifier_unregister(>notifier);
> v4l2_async_notifier_cleanup(>notifier);
> media_entity_cleanup(>vdev->entity);
>
Re: [PATCH] media: stm32-dcmi: Delete an unnecessary of_node_put() call in dcmi_probe()
Acked-by: Hugues Fruchet
On 8/27/19 4:09 PM, Markus Elfring wrote:
> From: Markus Elfring
> Date: Tue, 27 Aug 2019 16:00:13 +0200
>
> A null pointer would be passed to a call of the function “of_node_put”
> immediately after a call of the function “of_graph_get_next_endpoint”
> failed at one place.
> Remove this superfluous function call.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring
> ---
> drivers/media/platform/stm32/stm32-dcmi.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index cb1daf8217ff..9392e3409fba 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -1861,7 +1861,6 @@ static int dcmi_probe(struct platform_device *pdev)
> np = of_graph_get_next_endpoint(np, NULL);
> if (!np) {
> dev_err(>dev, "Could not find the endpoint\n");
> - of_node_put(np);
> return -ENODEV;
> }
>
> --
> 2.23.0
>
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.
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 "": when tracing subdev infos >> >> version 5: >>- Remove remaining Change-Id >>- Add Acked-by: Sakari Ailus >> >> 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(-) >> >
[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
---
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 *)_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 *)_fmts[num_fmts - 1]->fourcc,
+ sd_fmts[num_fmts - 1]->mbus_code);
+ }
}
mbus_code.index++;
}
--
2.7.4
[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 "": when tracing subdev infos version 5: - Remove remaining Change-Id - Add Acked-by: Sakari Ailus 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
[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
Signed-off-by: Hugues Fruchet
---
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_THRESHOLD3
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(, 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,
_cfg, );
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, );
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, );
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, get_fmt, NULL, );
if (ret)
return ret;
@@ -1020,7 +1020,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
}
v4l2_fill_mbus_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,
_cfg, );
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, );
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, );
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-&
[PATCH v7 3/4] media: stm32-dcmi: add media controller support
Add media controller support to dcmi in order
to walk within remote subdevices pipeline.
Signed-off-by: Hugues Fruchet
---
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 db01210..9293949 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 mutexdma_lock;
+
+ struct media_device mdev;
+ struct media_padvid_cap_pad;
};
static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier
*n)
@@ -1560,14 +1563,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;
}
@@ -1760,10 +1755,19 @@ static int dcmi_probe(struct platform_device *pdev)
q = >queue;
+ dcmi->v4l2_dev.mdev = >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 = >dev;
+ media_device_init(>mdev);
+
/* Initialize the top-level structure */
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
- goto err_dma_release;
+ goto err_media_device_cleanup;
dcmi->vdev = video_device_alloc();
if (!dcmi->vdev) {
@@ -1783,6 +1787,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(>vdev->entity,
+1, >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;
@@ -1798,12 +1821,12 @@ static int dcmi_probe(struct platform_device *pdev)
ret = vb2_queue_init(q);
if (ret < 0) {
dev_err(>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);
@@ -1830,11 +1853,14 @@ static int dcmi_probe(struct platform_device *pdev)
err_cleanup:
v4l2_async_notifier_cleanup(>notifier);
+err_media_entity_cleanup:
+ media_entity_cleanup(>vdev->entity);
err_device_release:
video_device_release(dcmi->vdev);
err_device_unregister:
v4l2_device_unregister(>v4l2_dev);
-err_dma_release:
+err_media_device_cleanup:
+ media_device_cleanup(>mdev);
dma_release_channel(dcmi->dma_chan);
return ret;
@@ -1848,7 +1874,9 @@ static int dcmi_remove(struct platform_device *pdev)
v4l2_async_notifier_unregister(>notifier);
v4l2_async_notifier_cleanup(>notifier);
+ media_entity_cleanup(>vdev->entity);
v4l2_device_unregister(>v4l2_dev);
+ media_device_cleanup(>mdev);
dma_release_channel(dcmi->dma_chan);
--
2.7.4
[PATCH v7 4/4] media: stm32-dcmi: add support of several sub-devices
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
---
drivers/media/platform/stm32/stm32-dcmi.c | 217 +++---
1 file changed, 198 insertions(+), 19 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 9293949..5bfbc07 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_padvid_cap_pad;
+ struct media_pipeline pipeline;
};
static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier
*n)
@@ -583,6 +584,144 @@ static void dcmi_buf_queue(struct vb2_buffer *vb)
spin_unlock_irq(>irqlock);
}
+static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi)
+{
+ struct media_entity *entity = >vdev->entity;
+ struct media_pad *pad;
+
+ /* Walk searching for entity having no sink */
+ while (1) {
+ pad = >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 = >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 = >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, );
+ if (ret < 0) {
+ dev_err(dcmi->dev, "%s: Failed to set format 0x%x %ux%u
on \"%s\":%d pad (%d)\n",
+ __func__, format->format.code,
+ format->format.width, format->format.height,
+ subdev->name, pad->index, ret);
+ return ret;
+ }
+
+ if (fmt.format.code != format->format.code ||
+ fmt.format.width != format->format.width ||
+ fmt.format.height != format->format.height) {
+ dev_dbg(dcmi->dev, "\"%s\":%d pad format has been
changed to 0x%x %ux%u\n",
+ subdev->name, pad->index, fmt.format.code,
+ fmt.format.width, fmt.format.height);
+ }
+
+ /* 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 = >vdev->entity;
+ struct v4l2_subdev *subdev;
+ struct media_pad *pad;
+ int ret;
+
+ /* Start/stop all entities within pipeline */
+ while (1) {
+ pad = >pads[0];
+ if (!(pad->flags & MEDIA_PAD_FL_SIN
Re: [PATCH v6 2/4] media: stm32-dcmi: trace the supported fourcc/mbus_code
Hi Sakari,
OK, I will change.
Have you some other remarks on this serie in order that I group changes
in the next version ?
On 8/19/19 9:26 AM, Sakari Ailus wrote:
> Hi Hugues,
>
> On Mon, Aug 19, 2019 at 07:23:17AM +0000, Hugues FRUCHET wrote:
>> Hi Sakari,
>>
>> On 8/16/19 10:15 AM, Sakari Ailus wrote:
>>> Hi Hugues,
>>>
>>> On Wed, Aug 14, 2019 at 03:48:51PM +0200, Hugues Fruchet wrote:
>>>> Add a trace of the set of supported fourcc/mbus_code which
>>>> intersect between DCMI and source sub-device.
>>>>
>>>> Signed-off-by: Hugues Fruchet
>>>> ---
>>>>drivers/media/platform/stm32/stm32-dcmi.c | 12 ++--
>>>>1 file changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
>>>> b/drivers/media/platform/stm32/stm32-dcmi.c
>>>> index b462f71..18acecf 100644
>>>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>>>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>>>> @@ -1447,12 +1447,20 @@ 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 *)_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",
>>>
>>> Over 80 characters per line.
>>>
>>
>> This an exception of the "80 chars" in order to be able to grep in
>> kernel messages:
>> https://www.kernel.org/doc/html/v4.10/process/coding-style.html
>> "However, never break user-visible strings such as printk messages,
>> because that breaks the ability to grep for them."
>>
>> This exception is managed in checkpatch.pl (--strict).
>
> This exception is for cases where wrapping the line in the usual way, e.g.
> at argument boundaries, does not prevent it exceeding 80 characters. But it is
> not the case here.
>
BR,
Hugues.
Re: [PATCH v6 2/4] media: stm32-dcmi: trace the supported fourcc/mbus_code
Hi Sakari,
On 8/16/19 10:15 AM, Sakari Ailus wrote:
> Hi Hugues,
>
> On Wed, Aug 14, 2019 at 03:48:51PM +0200, Hugues Fruchet wrote:
>> Add a trace of the set of supported fourcc/mbus_code which
>> intersect between DCMI and source sub-device.
>>
>> Signed-off-by: Hugues Fruchet
>> ---
>> drivers/media/platform/stm32/stm32-dcmi.c | 12 ++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
>> b/drivers/media/platform/stm32/stm32-dcmi.c
>> index b462f71..18acecf 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -1447,12 +1447,20 @@ 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 *)_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",
>
> Over 80 characters per line.
>
This an exception of the "80 chars" in order to be able to grep in
kernel messages:
https://www.kernel.org/doc/html/v4.10/process/coding-style.html
"However, never break user-visible strings such as printk messages,
because that breaks the ability to grep for them."
This exception is managed in checkpatch.pl (--strict).
>> +(char *)_fmts[num_fmts - 1]->fourcc,
>> +sd_fmts[num_fmts - 1]->mbus_code);
>> +}
>> }
>> mbus_code.index++;
>> }
>
BR,
Hugues.
Re: [PATCH v4 1/3] media: stm32-dcmi: improve sensor subdev naming
Hi Sakari, Hans, I've just pushed a v6 with the FIXME we discussed on IRC about "parallel" mbus code versus "serial" mbus code. I have also added some traces to help in debugging if such case occurs. 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 "": when tracing subdev infos Best regards, Hugues. On 8/9/19 6:01 PM, Sakari Ailus wrote: > Hi Hugues, > > Thanks for teh update. > > On Wed, Jul 31, 2019 at 02:56:21PM +0200, Hugues Fruchet wrote: >> 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 >> Change-Id: I1545a1a29a8061ee67cc6e4b799e9a69071911e7 > > No Change-Id tags in the kernel, please. Check the other two as well. > > With that fixed, > > Acked-by: Sakari Ailus >
[PATCH v6 3/4] media: stm32-dcmi: add media controller support
Add media controller support to dcmi in order
to walk within remote subdevices pipeline.
Signed-off-by: Hugues Fruchet
---
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 18acecf..302c40e 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 mutexdma_lock;
+
+ struct media_device mdev;
+ struct media_padvid_cap_pad;
};
static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier
*n)
@@ -1559,14 +1562,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;
}
@@ -1759,10 +1754,19 @@ static int dcmi_probe(struct platform_device *pdev)
q = >queue;
+ dcmi->v4l2_dev.mdev = >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 = >dev;
+ media_device_init(>mdev);
+
/* Initialize the top-level structure */
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
- goto err_dma_release;
+ goto err_media_device_cleanup;
dcmi->vdev = video_device_alloc();
if (!dcmi->vdev) {
@@ -1782,6 +1786,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(>vdev->entity,
+1, >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;
@@ -1797,12 +1820,12 @@ static int dcmi_probe(struct platform_device *pdev)
ret = vb2_queue_init(q);
if (ret < 0) {
dev_err(>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);
@@ -1829,11 +1852,14 @@ static int dcmi_probe(struct platform_device *pdev)
err_cleanup:
v4l2_async_notifier_cleanup(>notifier);
+err_media_entity_cleanup:
+ media_entity_cleanup(>vdev->entity);
err_device_release:
video_device_release(dcmi->vdev);
err_device_unregister:
v4l2_device_unregister(>v4l2_dev);
-err_dma_release:
+err_media_device_cleanup:
+ media_device_cleanup(>mdev);
dma_release_channel(dcmi->dma_chan);
return ret;
@@ -1847,7 +1873,9 @@ static int dcmi_remove(struct platform_device *pdev)
v4l2_async_notifier_unregister(>notifier);
v4l2_async_notifier_cleanup(>notifier);
+ media_entity_cleanup(>vdev->entity);
v4l2_device_unregister(>v4l2_dev);
+ media_device_cleanup(>mdev);
dma_release_channel(dcmi->dma_chan);
--
2.7.4
[PATCH v6 4/4] media: stm32-dcmi: add support of several sub-devices
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
---
drivers/media/platform/stm32/stm32-dcmi.c | 217 +++---
1 file changed, 198 insertions(+), 19 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 302c40e..b6ef229 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_padvid_cap_pad;
+ struct media_pipeline pipeline;
};
static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier
*n)
@@ -583,6 +584,144 @@ static void dcmi_buf_queue(struct vb2_buffer *vb)
spin_unlock_irq(>irqlock);
}
+static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi)
+{
+ struct media_entity *entity = >vdev->entity;
+ struct media_pad *pad;
+
+ /* Walk searching for entity having no sink */
+ while (1) {
+ pad = >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 = >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 = >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, );
+ if (ret < 0) {
+ dev_err(dcmi->dev, "%s: Failed to set format 0x%x %ux%u
on \"%s\":%d pad (%d)\n",
+ __func__, format->format.code,
+ format->format.width, format->format.height,
+ subdev->name, pad->index, ret);
+ return ret;
+ }
+
+ if (fmt.format.code != format->format.code ||
+ fmt.format.width != format->format.width ||
+ fmt.format.height != format->format.height) {
+ dev_dbg(dcmi->dev, "\"%s\":%d pad format has been
changed to 0x%x %ux%u\n",
+ subdev->name, pad->index, fmt.format.code,
+ fmt.format.width, fmt.format.height);
+ }
+
+ /* 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 = >vdev->entity;
+ struct v4l2_subdev *subdev;
+ struct media_pad *pad;
+ int ret;
+
+ /* Start/stop all entities within pipeline */
+ while (1) {
+ pad = >pads[0];
+ if (!(pad->flags & MEDIA_PAD_FL_SIN
[PATCH v6 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
---
drivers/media/platform/stm32/stm32-dcmi.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index b462f71..18acecf 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1447,12 +1447,20 @@ 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 *)_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 *)_fmts[num_fmts - 1]->fourcc,
+ sd_fmts[num_fmts - 1]->mbus_code);
+ }
}
mbus_code.index++;
}
--
2.7.4
[PATCH v6 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
Signed-off-by: Hugues Fruchet
---
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_THRESHOLD3
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(, 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,
_cfg, );
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, );
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, );
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, get_fmt, NULL, );
if (ret)
return ret;
@@ -1020,7 +1020,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
}
v4l2_fill_mbus_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,
_cfg, );
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, );
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, );
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-&
[PATCH v6 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 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 "": when tracing subdev infos version 5: - Remove remaining Change-Id - Add Acked-by: Sakari Ailus 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 | 317 +- 2 files changed, 267 insertions(+), 52 deletions(-) -- 2.7.4
[PATCH v5 1/3] 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
Signed-off-by: Hugues Fruchet
---
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_THRESHOLD3
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(, 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,
_cfg, );
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, );
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, );
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, get_fmt, NULL, );
if (ret)
return ret;
@@ -1020,7 +1020,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
}
v4l2_fill_mbus_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,
_cfg, );
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, );
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, );
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-&
[PATCH v5 0/3] 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 5: - Remove remaining Change-Id - Add Acked-by: Sakari Ailus 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 (3): media: stm32-dcmi: improve sensor subdev naming 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 | 283 +- 2 files changed, 236 insertions(+), 49 deletions(-) -- 2.7.4
[PATCH v5 3/3] media: stm32-dcmi: add support of several sub-devices
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.
Acked-by: Sakari Ailus
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 195 +++---
1 file changed, 177 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 6f37617..7c21805 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_padvid_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(>irqlock);
}
+static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi)
+{
+ struct media_entity *entity = >vdev->entity;
+ struct media_pad *pad;
+
+ /* Walk searching for entity having no sink */
+ while (1) {
+ pad = >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 = >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 = >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, );
+ 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 = >vdev->entity;
+ struct v4l2_subdev *subdev;
+ struct media_pad *pad;
+ int ret;
+
+ /* Start/stop all entities within pipeline */
+ while (1) {
+ pad = >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 ? "starte
[PATCH v5 2/3] media: stm32-dcmi: add media controller support
Add media controller support to dcmi in order
to walk within remote subdevices pipeline.
Acked-by: Sakari Ailus
Signed-off-by: Hugues Fruchet
---
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 mutexdma_lock;
+
+ struct media_device mdev;
+ struct media_padvid_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 = >queue;
+ dcmi->v4l2_dev.mdev = >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 = >dev;
+ media_device_init(>mdev);
+
/* Initialize the top-level structure */
ret = v4l2_device_register(>dev, >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(>vdev->entity,
+1, >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(>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(>notifier);
+err_media_entity_cleanup:
+ media_entity_cleanup(>vdev->entity);
err_device_release:
video_device_release(dcmi->vdev);
err_device_unregister:
v4l2_device_unregister(>v4l2_dev);
-err_dma_release:
+err_media_device_cleanup:
+ media_device_cleanup(>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(>notifier);
v4l2_async_notifier_cleanup(>notifier);
+ media_entity_cleanup(>vdev->entity);
v4l2_device_unregister(>v4l2_dev);
+ media_device_cleanup(>mdev);
dma_release_channel(dcmi->dma_chan);
--
2.7.4
Re: [PATCH v4 1/3] media: stm32-dcmi: improve sensor subdev naming
Hi Sakari, Sorry for inconvenience, I will push a V5 with that fixed. May I put your "Acked-by" also on the 2 other commits ? Or just this one ? On 8/9/19 6:01 PM, Sakari Ailus wrote: > Hi Hugues, > > Thanks for teh update. > > On Wed, Jul 31, 2019 at 02:56:21PM +0200, Hugues Fruchet wrote: >> 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 >> Change-Id: I1545a1a29a8061ee67cc6e4b799e9a69071911e7 > > No Change-Id tags in the kernel, please. Check the other two as well. > > With that fixed, > > Acked-by: Sakari Ailus > Best regards, Hugues.
Re: [PATCH v4 0/3] DCMI bridge support
Hi Hans,
On 8/8/19 11:41 AM, Hans Verkuil wrote:
> Hi Hugues,
>
> On 8/8/19 11:38 AM, Hugues FRUCHET wrote:
>> Hi Hans,
>>
>> You're welcome, here it is:
>>
>> 1) v4l-utils master branch, commit
>> 6aa15f7447d4aeca6af1ed7ee9644a0c7e891ece "v4l2-ctl: fix double
>> decrementing of stream_count"
>>
>> 2) Cropping test is failed as usual because of OV5640 discrete framesizes
>>
>> 3) No more /dev/media* and /dev/v4l-*:
>> root@stm32mp1-av96:~# ls -al /dev/video0
>> crw-rw 1 root video 81, 0 Mar 19 17:42 /dev/video0
>> root@stm32mp1-av96:~# ls -al /dev/media*
>> ls: cannot access '/dev/media*': No such file or directory
>> root@stm32mp1-av96:~# ls -al /dev/v4l-*
>> ls: cannot access '/dev/v4l-*': No such file or directory
>
> Good. One more question: is this tested with two subdevs? So a bridge+sensor?
Yes, tested with ov5640 (CSI) => st-mipid02 => stm32-dcmi.
In term of hardware setup, it's an Avenger96 96 board [1] embedding an
STM32MP157 (DCMI //) and an ST MIPID02 bridge (CSI to //) in order to
enable CSI video input on high speed expansion connector + a D3
engineering mezzanine board [2] embedding an OV5640 CSI camera.
{
...
port {
dcmi_0: endpoint {
remote-endpoint = <_2>;
mipid02: mipid02@14 {
...
port@0 {
reg = <0>;
mipid02_0: endpoint {
data-lanes = <1 2>;
remote-endpoint = <_0>;
};
};
port@2 {
reg = <2>;
mipid02_2: endpoint {
bus-width = <8>;
hsync-active = <0>;
vsync-active = <0>;
pclk-sample = <0>;
remote-endpoint = <_0>;
};
ov5640: camera@3c {
...
ov5640_0: endpoint {
remote-endpoint = <_0>;
clock-lanes = <0>;
data-lanes = <1 2>;
};
[1]
https://www.96boards.org/product/avenger96/
https://wiki.dh-electronics.com/index.php/Avenger96
[2] https://www.96boards.org/product/d3camera/
>
> Regards,
>
> Hans
>
>>
>>
>> root@stm32mp1-av96:~# v4l2-compliance -s
>> v4l2-compliance SHA: 6aa15f7447d4aeca6af1ed7ee9644a0c7e891ece, 32 bits
>>
>> Compliance test for stm32-dcmi device /dev/video0:
>>
>> Driver Info:
>> Driver name : stm32-dcmi
>> Card type: STM32 Camera Memory Interface
>> Bus info : platform:dcmi
>> Driver version : 4.19.49
>> Capabilities : 0x8521
>> Video Capture
>> Read/Write
>> Streaming
>> Extended Pix Format
>> Device Capabilities
>> Device Caps : 0x0521
>> Video Capture
>> Read/Write
>> Streaming
>> Extended Pix Format
>>
>> Required ioctls:
>> test VIDIOC_QUERYCAP: OK
>>
>> Allow for multiple opens:
>> test second /dev/video0 open: OK
>> test VIDIOC_QUERYCAP: OK
>> test VIDIOC_G/S_PRIORITY: OK
>> test for unlimited opens: OK
>>
>> Debug ioctls:
>> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>> test VIDIOC_LOG_STATUS: OK
>>
>> Input ioctls:
>> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>> test VIDIOC_ENUMAUDIO: OK (Not Supported)
>> test VIDIOC_G/S/ENUMINPUT: OK
>> test VIDIOC_G/S_AUDIO: OK (Not Supported)
>> Inputs: 1 Audio Inputs: 0 Tuners: 0
>>
>> Output ioctls:
>> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>> Outputs: 0 Audio Outputs:
Re: [PATCH v4 0/3] DCMI bridge support
Hi Hans, You're welcome, here it is: 1) v4l-utils master branch, commit 6aa15f7447d4aeca6af1ed7ee9644a0c7e891ece "v4l2-ctl: fix double decrementing of stream_count" 2) Cropping test is failed as usual because of OV5640 discrete framesizes 3) No more /dev/media* and /dev/v4l-*: root@stm32mp1-av96:~# ls -al /dev/video0 crw-rw 1 root video 81, 0 Mar 19 17:42 /dev/video0 root@stm32mp1-av96:~# ls -al /dev/media* ls: cannot access '/dev/media*': No such file or directory root@stm32mp1-av96:~# ls -al /dev/v4l-* ls: cannot access '/dev/v4l-*': No such file or directory root@stm32mp1-av96:~# v4l2-compliance -s v4l2-compliance SHA: 6aa15f7447d4aeca6af1ed7ee9644a0c7e891ece, 32 bits Compliance test for stm32-dcmi device /dev/video0: Driver Info: Driver name : stm32-dcmi Card type: STM32 Camera Memory Interface Bus info : platform:dcmi Driver version : 4.19.49 Capabilities : 0x8521 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x0521 Video Capture Read/Write Streaming Extended Pix Format Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video0 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls (Input 0): test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 18 Private Controls: 0 Format ioctls (Input 0): test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) fail: ../../../../../../../../../sources/v4l-utils/utils/v4l2-compliance/v4l2-test-formats.cpp(1414): node->frmsizes_count[pixfm t] > 1 test Cropping: FAIL test Composing: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls (Input 0): test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls (Input 0): test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK test blocking wait: OK test MMAP (no poll): OK test MMAP (select): OK test MMAP (epoll): OK test USERPTR (no poll): OK (Not Supported) test USERPTR (select): OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total for stm32-dcmi device /dev/video0: 51, Succeeded: 50, Failed: 1, Warnings: 0 On 8/7/19 12:15 PM, Hans Verkuil wrote: > Hi Hugues, > > Can you provide the output of the most recent v4l2-compliance? > > Use 'v4l2-compliance -s'. > > Also, just to confirm, with this v4 there are no /dev/mediaX or > /dev/v4l-subdevX devices created anymore, right? > > This v4 looks good to me, I just want to have these final checks > done. > > Regards, > > Hans > Best regards, Hugues.
Re: [PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices
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 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
>>>> ---
>>>> 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_padvid_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(>irqlock);
>>>> }
>>>>
>>>> +static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi)
>>>> +{
>>>> + struct media_entity *entity = >vdev->entity;
>>>> + struct media_pad *pad;
>>>> +
>>>> + /* Walk searching for entity having no sink */
>>>> + while (1) {
>>>> + pad = >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 = >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 = >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
[PATCH v4 1/3] 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.
Signed-off-by: Hugues Fruchet
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_THRESHOLD3
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(, 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,
_cfg, );
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, );
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, );
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, get_fmt, NULL, );
if (ret)
return ret;
@@ -1020,7 +1020,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
}
v4l2_fill_mbus_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,
_cfg, );
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, );
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, );
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
[PATCH v4 0/3] 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 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 (3): media: stm32-dcmi: improve sensor subdev naming 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 | 283 +- 2 files changed, 236 insertions(+), 49 deletions(-) -- 2.7.4
[PATCH v4 2/3] media: stm32-dcmi: add media controller support
Add media controller support to dcmi in order
to walk within remote subdevices pipeline.
Signed-off-by: Hugues Fruchet
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 mutexdma_lock;
+
+ struct media_device mdev;
+ struct media_padvid_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 = >queue;
+ dcmi->v4l2_dev.mdev = >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 = >dev;
+ media_device_init(>mdev);
+
/* Initialize the top-level structure */
ret = v4l2_device_register(>dev, >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(>vdev->entity,
+1, >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(>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(>notifier);
+err_media_entity_cleanup:
+ media_entity_cleanup(>vdev->entity);
err_device_release:
video_device_release(dcmi->vdev);
err_device_unregister:
v4l2_device_unregister(>v4l2_dev);
-err_dma_release:
+err_media_device_cleanup:
+ media_device_cleanup(>mdev);
dma_release_channel(dcmi->dma_chan);
return ret;
@@ -1839,7 +1865,9 @@ static int dcmi_remove(struct platform_device *pdev)
v4l2_async_notifi
[PATCH v4 3/3] media: stm32-dcmi: add support of several sub-devices
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
---
drivers/media/platform/stm32/stm32-dcmi.c | 195 +++---
1 file changed, 177 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 6f37617..7c21805 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_padvid_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(>irqlock);
}
+static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi)
+{
+ struct media_entity *entity = >vdev->entity;
+ struct media_pad *pad;
+
+ /* Walk searching for entity having no sink */
+ while (1) {
+ pad = >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 = >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 = >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, );
+ 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 = >vdev->entity;
+ struct v4l2_subdev *subdev;
+ struct media_pad *pad;
+ int ret;
+
+ /* Start/stop all entities within pipeline */
+ while (1) {
+ pad = >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&qu
[PATCH v3 1/3] 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.
Signed-off-by: Hugues Fruchet
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_THRESHOLD3
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(, 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,
_cfg, );
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, );
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, );
+ ret = v4l2_subdev_call(dcmi->entity.source, pad, get_fmt, NULL, );
if (ret)
return ret;
@@ -1020,7 +1020,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
}
v4l2_fill_mbus_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,
_cfg, );
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, );
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, );
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
[PATCH v3 3/3] media: stm32-dcmi: add support of several sub-devices
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
---
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_padvid_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(>irqlock);
}
+static struct media_entity *dcmi_find_source(struct stm32_dcmi *dcmi)
+{
+ struct media_entity *entity = >vdev->entity;
+ struct media_pad *pad;
+
+ /* Walk searching for entity having no sink */
+ while (1) {
+ pad = >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 = >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 = >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, );
+ 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 = >vdev->entity;
+ struct v4l2_subdev *subdev;
+ struct media_pad *pad;
+ int ret;
+
+ /* Start/stop all entities within pipeline */
+ while (1) {
+ pad = >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&qu
[PATCH v3 2/3] media: stm32-dcmi: add media controller support
Add media controller support to dcmi in order
to walk within remote subdevices pipeline.
Signed-off-by: Hugues Fruchet
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 mutexdma_lock;
+
+ struct media_device mdev;
+ struct media_padvid_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 = >queue;
+ dcmi->v4l2_dev.mdev = >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 = >dev;
+ media_device_init(>mdev);
+
/* Initialize the top-level structure */
ret = v4l2_device_register(>dev, >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(>vdev->entity,
+1, >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(>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(>notifier);
+err_media_entity_cleanup:
+ media_entity_cleanup(>vdev->entity);
err_device_release:
video_device_release(dcmi->vdev);
err_device_unregister:
v4l2_device_unregister(>v4l2_dev);
-err_dma_release:
+err_media_device_cleanup:
+ media_device_cleanup(>mdev);
dma_release_channel(dcmi->dma_chan);
return ret;
@@ -1839,7 +1865,9 @@ static int dcmi_remove(struct platform_device *pdev)
v4l2_async_notifi
[PATCH v3 0/3] 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 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 (3): media: stm32-dcmi: improve sensor subdev naming 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 | 294 +- 2 files changed, 247 insertions(+), 49 deletions(-) -- 2.7.4
Re: [PATCH v2 2/3] media: stm32-dcmi: add media controller support
Hi Sakari,
On 6/20/19 6:13 PM, Sakari Ailus wrote:
> Hi Hugues,
>
> On Tue, Jun 11, 2019 at 10:48:31AM +0200, Hugues Fruchet wrote:
>> Add media controller support to dcmi.
>>
>> Signed-off-by: Hugues Fruchet
>> ---
>> drivers/media/platform/Kconfig| 2 +-
>> drivers/media/platform/stm32/stm32-dcmi.c | 83
>> +++
>> 2 files changed, 63 insertions(+), 22 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
>
> Ok, if the intent is to require MC from now on, then I think you could
> simply rely on media_entity_remote_pad() in finding the image source.
>
>> 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 7a4d559..3a69783 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -170,6 +170,9 @@ struct stm32_dcmi {
>>
>> /* Ensure DMA operations atomicity */
>> struct mutexdma_lock;
>> +
>> +struct media_device mdev;
>> +struct media_padvid_cap_pad;
>> };
>>
>> static inline struct stm32_dcmi *notifier_to_dcmi(struct
>> v4l2_async_notifier *n)
>> @@ -1545,21 +1548,12 @@ static int dcmi_graph_notify_complete(struct
>> v4l2_async_notifier *notifier)
>> dev_err(dcmi->dev, "Could not get sensor bounds\n");
>> return ret;
>> }
>> -
>> ret = dcmi_set_default_fmt(dcmi);
>> if (ret) {
>> dev_err(dcmi->dev, "Could not set default format\n");
>> 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;
>> }
>>
>> @@ -1648,6 +1642,12 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi)
>> return 0;
>> }
>>
>> +static void dcmi_graph_deinit(struct stm32_dcmi *dcmi)
>> +{
>> +v4l2_async_notifier_unregister(>notifier);
>> +v4l2_async_notifier_cleanup(>notifier);
>
> I'd just leave the calls where they are now. This doesn't improve
> readability of the code, rather the opposite.
>
fixed in v3.
>> +}
>> +
>> static int dcmi_probe(struct platform_device *pdev)
>> {
>> struct device_node *np = pdev->dev.of_node;
>> @@ -1752,10 +1752,27 @@ static int dcmi_probe(struct platform_device *pdev)
>>
>> q = >queue;
>>
>> +dcmi->v4l2_dev.mdev = >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 = >dev;
>> +media_device_init(>mdev);
>> +
>> +/* Register the media device */
>> +ret = media_device_register(>mdev);
>> +if (ret) {
>> +dev_err(dcmi->dev, "Failed to register media device (%d)\n",
>> +ret);
>> +goto err_media_device_cleanup;
>> +}
>> +
>> /* Initialize the top-level structure */
>> ret = v4l2_device_register(>dev, >v4l2_dev);
>> if (ret)
>> -goto err_dma_release;
>> +goto err_media_device_unregister;
>>
>> dcmi->vdev = video_device_alloc();
>> if (!dcmi->vdev) {
>> @@ -1775,6 +1792,25 @@ static int dcmi_probe(struct platform_device *pdev)
>>V4L2_CAP_READWRITE;
>> video_set_drvdata(dcmi->v
Re: [PATCH v2 3/3] media: stm32-dcmi: add support of several sub-devices
Hi Sakari,
On 6/20/19 5:54 PM, Sakari Ailus wrote:
> Hi Hugues,
>
> Thank you for the update.
>
> On Tue, Jun 11, 2019 at 10:48:32AM +0200, 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
>> ---
>> drivers/media/platform/stm32/stm32-dcmi.c | 207
>> +++---
>> 1 file changed, 189 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
>> b/drivers/media/platform/stm32/stm32-dcmi.c
>> index 3a69783..144912f 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -173,6 +173,7 @@ struct stm32_dcmi {
>>
>> struct media_device mdev;
>> struct media_padvid_cap_pad;
>> +struct media_pipeline pipeline;
>> };
>>
>> static inline struct stm32_dcmi *notifier_to_dcmi(struct
>> v4l2_async_notifier *n)
>> @@ -584,6 +585,135 @@ static void dcmi_buf_queue(struct vb2_buffer *vb)
>> spin_unlock_irq(>irqlock);
>> }
>>
>> +static struct media_entity *dcmi_find_sensor(struct stm32_dcmi *dcmi)
>
> You generally should be only concerned with the next entity connected to the
> one you're in control of, not the rest of the pipeline.
This was discussed with Laurent here:
https://www.spinics.net/lists/linux-media/msg153417.html
and it's OK because DCMI is a video node and we are not
exposing media controller interface to userspace.
>
>> +{
>> +struct media_entity *entity = >vdev->entity;
>> +struct v4l2_subdev *subdev;
>> +struct media_pad *pad;
>> +
>> +/* Walk searching for entity having no sink */
>> +while (1) {
>> +pad = >pads[0];
>> +
>> +subdev = media_entity_to_v4l2_subdev(entity);
>> +
>> +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 = >sensor->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 = >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, );
>
> Generally speaking, on MC-centric devices, the user space needs to
> configure the pipeline. The driver's responsibility is to validate it
> (through the link_validate media entity and subdev pad ops). I.e. set_fmt
> is only used through the subdev nodes.
>
ditto
>> +
Re: [PATCH v2 1/3] media: stm32-dcmi: improve sensor subdev naming
Hi Sakari, On 6/20/19 5:26 PM, Sakari Ailus wrote: > Hi Hugues, > > On Tue, Jun 11, 2019 at 10:48:30AM +0200, Hugues Fruchet wrote: >> Add a new "sensor" field to dcmi struct instead of >> reusing entity->subdev to address sensor subdev. As discussed on IRC, fixed in v3, > > The purpose of the struct binding image source's async subdev as well as > related information is to allow associating the two. This patch breaks > that. If your device can support a single sensor, it might not be a big > deal. The end result remains somewhat inconsistent as subdev specific > information is spread across struct stm32_dcmi and struct > dcmi_graph_entity. As discussed on IRC, fixed in v3, > > In general you don't need to know the sensor as you can always find it > using media_entity_remote_pad(). This driver is a little different though > as it could presumably continue to work without MC. Was that the intent? > > On a side note: struct dcmi_graph_entity does NOT have struct > v4l2_async_subdev as its first member. Please fix that and prepend the fix > to this set. > As discussed on IRC, fixed in v3, BR, Hugues.
Re: [PATCH v2 2/3] media: stm32-dcmi: add media controller support
Hi Hans,
On 6/20/19 2:01 PM, Hans Verkuil wrote:
> On 6/11/19 10:48 AM, Hugues Fruchet wrote:
>> Add media controller support to dcmi.
>>
>> Signed-off-by: Hugues Fruchet
>> ---
>> drivers/media/platform/Kconfig| 2 +-
>> drivers/media/platform/stm32/stm32-dcmi.c | 83
>> +++
>> 2 files changed, 63 insertions(+), 22 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 7a4d559..3a69783 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -170,6 +170,9 @@ struct stm32_dcmi {
>>
>> /* Ensure DMA operations atomicity */
>> struct mutexdma_lock;
>> +
>> +struct media_device mdev;
>> +struct media_padvid_cap_pad;
>> };
>>
>> static inline struct stm32_dcmi *notifier_to_dcmi(struct
>> v4l2_async_notifier *n)
>> @@ -1545,21 +1548,12 @@ static int dcmi_graph_notify_complete(struct
>> v4l2_async_notifier *notifier)
>> dev_err(dcmi->dev, "Could not get sensor bounds\n");
>> return ret;
>> }
>> -
>> ret = dcmi_set_default_fmt(dcmi);
>> if (ret) {
>> dev_err(dcmi->dev, "Could not set default format\n");
>> 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;
>> -}
>
> Why was this moved to probe()? Off-hand I see no reason for that.
>
> Regards,
>
> Hans
>
I need to do that otherwise the dcmi_graph_notify_bound() subdevice pad
link code crashes:
+ /*
+ * Link this sub-device to DCMI, it could be
+ * a parallel camera sensor or a bridge
+ */
+ src_pad = media_entity_get_fwnode_pad(>entity,
+ subdev->fwnode,
+ MEDIA_PAD_FL_SOURCE);
+
+ ret = media_create_pad_link(>entity, src_pad,
+ >vdev->entity, 0,
+ MEDIA_LNK_FL_IMMUTABLE |
+ MEDIA_LNK_FL_ENABLED);
see https://www.spinics.net/lists/linux-media/msg153120.html.
>> -
>> -dev_dbg(dcmi->dev, "Device registered as %s\n",
>> -video_device_node_name(dcmi->vdev));
>> return 0;
>> }
>>
>> @@ -1648,6 +1642,12 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi)
>> return 0;
>> }
>>
>> +static void dcmi_graph_deinit(struct stm32_dcmi *dcmi)
>> +{
>> +v4l2_async_notifier_unregister(>notifier);
>> +v4l2_async_notifier_cleanup(>notifier);
>> +}
>> +
>> static int dcmi_probe(struct platform_device *pdev)
>> {
>> struct device_node *np = pdev->dev.of_node;
>> @@ -1752,10 +1752,27 @@ static int dcmi_probe(struct platform_device *pdev)
>>
>> q = >queue;
>>
>> +dcmi->v4l2_dev.mdev = >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 = >dev;
>> +media_device_init(>mdev);
>> +
>> +/* Register the media device */
>> +ret = media_device_register(>mdev);
>> +if (ret) {
>> +dev_err(dcmi->dev, "Failed to register media device (%d)\n",
>> +ret);
>> +goto err_media_device_cleanup;
>> +}
>> +
>> /* Initialize the top-level structur
Re: [PATCH v2 0/3] DCMI bridge support
Hi Laurent,
Thanks for reviewing,
On 6/26/19 7:25 PM, Laurent Pinchart wrote:
> Hi Hugues,
>
> On Mon, Jun 24, 2019 at 10:10:05AM +0000, Hugues FRUCHET wrote:
>> Hi Sakari,
>>
>> > - Where's the sub-device representing the bridge itself?
>> This is pointed by [1]: drivers/media/i2c/st-mipid02.c
>>
>> > - As the driver becomes MC-centric, crop configuration takes place
>> through
>> > V4L2 sub-device interface, not through the video device node.
>> > - Same goes for accessing sensor configuration: it does not take place
>> > through video node but through the sub-device nodes.
>>
>> Our objective is to be able to support either a simple parallel sensor
>> or a CSI-2 sensor connected through a bridge without any changes on
>> userspace side because no additional processing or conversion involved,
>> only deserialisation is m.
>> With the proposed set of patches, we succeeded to do so, the same
>> non-regression tests campaign is passed with OV5640 parallel sensor
>> (STM32MP1 evaluation board) or OV5640 CSI-2 sensor (Avenger96 board with
>> D3 mezzanine board).
>>
>> We don't want driver to be MC-centric, media controller support was
>> required only to get access to the set of functions needed to link and
>> walk trough subdevices: media_create_pad_link(),
>> media_entity_remote_pad(), etc...
>>
>> We did a try with the v1 version of this patchset, delegating subdevices
>> handling to userspace, by using media-controller, but this require to
>> configure first the pipeline for each single change of resolution and
>> format before making any capture using v4l2-ctl or GStreamer, quite
>> heavy in fact.
>> Benjamin did another try using new libcamera codebase, but even for a
>> basic capture use-case, negotiation code is quite tricky in order to
>> match the right subdevices bus format to the required V4L2 format.
>
> Why would it be trickier in userspace than in the kernel ? The V4L2
> subdev operations are more or less expose verbatim through the subdev
> userspace API.
>
>> Moreover, it was not clear how to call libcamera library prior to any
>> v4l2-ctl or GStreamer calls.
>
> libcamera isn't meant to be called before v4l2-ctl or GStreamer.
> Applications are supposed to be based directly on libcamera, or, for
> existing userspace APIs such as V4L2 or GStreamer, compatibility layers
> are supposed to be developed. For V4L2 it will take the form of a
> LD_PRELOAD-able .so that will intercept the V4L2 API calls, making most
> V4L2 applications work with libcamera unmodified (I said most as 100%
> compatibility will likely not be achievable). For GStreamer it will take
> the form of a GStreamer libcamera element that will replace the V4L2
> source element.
>
>> Adding 100 lines of code into DCMI to well configure resolution and
>> formats fixes the point and allows us to keep backward compatibility
>> as per our objective, so it seems far more reasonable to us to do so
>> even if DCMI controls more than the subdevice it is connected to.
>> Moreover we found similar code in other video interfaces code like
>> qcom/camss/camss.c and xilinx/xilinx-dma.c, controlling the whole
>> pipeline, so it seems to us quite natural to go this way.
>
> I can't comment on the qcom-camss driver as I'm not aware of its
> internals, but where have you found such code in the Xilinx V4L2 drivers
> ?
For ex. in xilinx/xilinx-dma.c, stream on/off is propagated to all
subdevices within pipeline:
* Walk the entities chain starting at the pipeline output video node
static int xvip_pipeline_start_stop(struct xvip_pipeline *pipe, bool start)
Same for qcom/camss/camss-video.c:
static int video_start_streaming(struct vb2_queue *q, unsigned int count)
For resolution/format, in exynos4-is/fimc-capture.c:
static int fimc_pipeline_try_format(struct fimc_ctx *ctx,
...
while (1) {
...
/* set format on all pipeline subdevs */
while (me != >vid_cap.subdev.entity) {
...
ret = v4l2_subdev_call(sd, pad, set_fmt, NULL, );
>
>> To summarize, if we cannot do the negotiation within kernel, delegating
>> this to userspace implies far more complexity and breaks compatibility
>> with existing applications without adding new functionalities.
>>
>> Having all that in mind, what should be reconsidered in your opinion
>> Sakari ? Do you have some alternatives ?
>
> First of all, let's note that your patch series performs to related but
> still independent changes: it enables MC support, *and* enables the V4L2
> subdev userspace API. The former is clearly needed and will allow
Re: [PATCH v2 0/3] DCMI bridge support
Hi Sakari, > - Where's the sub-device representing the bridge itself? This is pointed by [1]: drivers/media/i2c/st-mipid02.c > - As the driver becomes MC-centric, crop configuration takes place through > V4L2 sub-device interface, not through the video device node. > - Same goes for accessing sensor configuration: it does not take place > through video node but through the sub-device nodes. Our objective is to be able to support either a simple parallel sensor or a CSI-2 sensor connected through a bridge without any changes on userspace side because no additional processing or conversion involved, only deserialisation is m. With the proposed set of patches, we succeeded to do so, the same non-regression tests campaign is passed with OV5640 parallel sensor (STM32MP1 evaluation board) or OV5640 CSI-2 sensor (Avenger96 board with D3 mezzanine board). We don't want driver to be MC-centric, media controller support was required only to get access to the set of functions needed to link and walk trough subdevices: media_create_pad_link(), media_entity_remote_pad(), etc... We did a try with the v1 version of this patchset, delegating subdevices handling to userspace, by using media-controller, but this require to configure first the pipeline for each single change of resolution and format before making any capture using v4l2-ctl or GStreamer, quite heavy in fact. Benjamin did another try using new libcamera codebase, but even for a basic capture use-case, negotiation code is quite tricky in order to match the right subdevices bus format to the required V4L2 format. Moreover, it was not clear how to call libcamera library prior to any v4l2-ctl or GStreamer calls. Adding 100 lines of code into DCMI to well configure resolution and formats fixes the point and allows us to keep backward compatibility as per our objective, so it seems far more reasonable to us to do so even if DCMI controls more than the subdevice it is connected to. Moreover we found similar code in other video interfaces code like qcom/camss/camss.c and xilinx/xilinx-dma.c, controlling the whole pipeline, so it seems to us quite natural to go this way. To summarize, if we cannot do the negotiation within kernel, delegating this to userspace implies far more complexity and breaks compatibility with existing applications without adding new functionalities. Having all that in mind, what should be reconsidered in your opinion Sakari ? Do you have some alternatives ? Best regards, Hugues. On 6/20/19 6:17 PM, Sakari Ailus wrote: > Hi Hugues, > > On Tue, Jun 11, 2019 at 10:48:29AM +0200, 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 > > General notes on the set, not related to any single patch: > > - Where's the sub-device representing the bridge itself? > > - As the driver becomes MC-centric, crop configuration takes place through >V4L2 sub-device interface, not through the video device node. > > - Same goes for accessing sensor configuration: it does not take place >through video node but through the sub-device nodes. >
[PATCH v2 3/3] media: st-mipid02: add support of JPEG
Add support of JPEG pixel format.
This requires auto detection of data type from CSI-2 stream.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/st-mipid02.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 35946ad..ff8a4d8 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -64,6 +64,7 @@ static const u32 mipid02_supported_fmt_codes[] = {
MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_BGR888_1X24,
MEDIA_BUS_FMT_RGB565_2X8_LE, MEDIA_BUS_FMT_RGB565_2X8_BE,
MEDIA_BUS_FMT_YUYV8_2X8, MEDIA_BUS_FMT_UYVY8_2X8,
+ MEDIA_BUS_FMT_JPEG_1X8
};
/* regulator supplies */
@@ -101,6 +102,7 @@ struct mipid02_dev {
u8 data_lane1_reg1;
u8 mode_reg1;
u8 mode_reg2;
+ u8 data_selection_ctrl;
u8 data_id_rreg;
u8 pix_width_ctrl;
u8 pix_width_ctrl_emb;
@@ -463,6 +465,7 @@ static int mipid02_configure_from_tx(struct mipid02_dev
*bridge)
{
struct v4l2_fwnode_endpoint *ep = >tx;
+ bridge->r.data_selection_ctrl = SELECTION_MANUAL_WIDTH;
bridge->r.pix_width_ctrl = ep->bus.parallel.bus_width;
bridge->r.pix_width_ctrl_emb = ep->bus.parallel.bus_width;
if (ep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
@@ -478,10 +481,15 @@ static int mipid02_configure_from_code(struct mipid02_dev
*bridge)
u8 data_type;
bridge->r.data_id_rreg = 0;
- data_type = data_type_from_code(bridge->fmt.code);
- if (!data_type)
- return -EINVAL;
- bridge->r.data_id_rreg = data_type;
+
+ if (bridge->fmt.code != MEDIA_BUS_FMT_JPEG_1X8) {
+ bridge->r.data_selection_ctrl |= SELECTION_MANUAL_DATA;
+
+ data_type = data_type_from_code(bridge->fmt.code);
+ if (!data_type)
+ return -EINVAL;
+ bridge->r.data_id_rreg = data_type;
+ }
return 0;
}
@@ -565,7 +573,7 @@ static int mipid02_stream_enable(struct mipid02_dev *bridge)
if (ret)
goto error;
ret = mipid02_write_reg(bridge, MIPID02_DATA_SELECTION_CTRL,
- SELECTION_MANUAL_DATA | SELECTION_MANUAL_WIDTH);
+ bridge->r.data_selection_ctrl);
if (ret)
goto error;
ret = mipid02_write_reg(bridge, MIPID02_PIX_WIDTH_CTRL,
--
2.7.4
[PATCH] media: st-mipid02: add support of V4L2_CID_LINK_FREQ
Ask device connected on sink pad for link frequency
in order to configure CLK_LANE_REG1 (ui_x4).
If not available, ask for pixel rate information to compute it.
This is needed to deal with compressed format such as JPEG
where number of bits per pixel is unknown: computation of
link frequency from pixel rate is not possible.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/st-mipid02.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 9369f38..8623f30 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -331,6 +331,25 @@ static int mipid02_detect(struct mipid02_dev *bridge)
return mipid02_read_reg(bridge, MIPID02_CLK_LANE_WR_REG1, );
}
+static u32 mipid02_get_link_freq_from_cid_link_freq(struct mipid02_dev *bridge,
+ struct v4l2_subdev *subdev)
+{
+ struct v4l2_querymenu qm = {.id = V4L2_CID_LINK_FREQ, };
+ struct v4l2_ctrl *ctrl;
+ int ret;
+
+ ctrl = v4l2_ctrl_find(subdev->ctrl_handler, V4L2_CID_LINK_FREQ);
+ if (!ctrl)
+ return 0;
+ qm.index = v4l2_ctrl_g_ctrl(ctrl);
+
+ ret = v4l2_querymenu(subdev->ctrl_handler, );
+ if (ret)
+ return 0;
+
+ return qm.value;
+}
+
static u32 mipid02_get_link_freq_from_cid_pixel_rate(struct mipid02_dev
*bridge,
struct v4l2_subdev *subdev)
{
@@ -358,10 +377,14 @@ static int mipid02_configure_from_rx_speed(struct
mipid02_dev *bridge)
struct v4l2_subdev *subdev = bridge->s_subdev;
u32 link_freq;
- link_freq = mipid02_get_link_freq_from_cid_pixel_rate(bridge, subdev);
+ link_freq = mipid02_get_link_freq_from_cid_link_freq(bridge, subdev);
if (!link_freq) {
- dev_err(>dev, "Failed to detect link frequency");
- return -EINVAL;
+ link_freq = mipid02_get_link_freq_from_cid_pixel_rate(bridge,
+ subdev);
+ if (!link_freq) {
+ dev_err(>dev, "Failed to get link frequency");
+ return -EINVAL;
+ }
}
dev_dbg(>dev, "detect link_freq = %d Hz", link_freq);
--
2.7.4
[PATCH v2 0/3] Add support of RGB565, YUV and JPEG to MIPID02 bridge
Add support of RGB565, YUV and JPEG to MIPID02 bridge. === = history = === version 2: - Link frequency could not be computed from pixel rate for JPEG, remove JPEG case in bpp_from_code(). version 1: - Initial submission Hugues Fruchet (3): media: st-mipid02: add support of RGB565 media: st-mipid02: add support of YUYV8 and UYVY8 media: st-mipid02: add support of JPEG drivers/media/i2c/st-mipid02.c | 31 +-- 1 file changed, 25 insertions(+), 6 deletions(-) -- 2.7.4
[PATCH v2 2/3] media: st-mipid02: add support of YUYV8 and UYVY8
Add support of YUYV8 and UYVY8 pixel formats.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/st-mipid02.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 76a9f52..35946ad 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -63,6 +63,7 @@ static const u32 mipid02_supported_fmt_codes[] = {
MEDIA_BUS_FMT_SGRBG12_1X12, MEDIA_BUS_FMT_SRGGB12_1X12,
MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_BGR888_1X24,
MEDIA_BUS_FMT_RGB565_2X8_LE, MEDIA_BUS_FMT_RGB565_2X8_BE,
+ MEDIA_BUS_FMT_YUYV8_2X8, MEDIA_BUS_FMT_UYVY8_2X8,
};
/* regulator supplies */
@@ -129,6 +130,8 @@ static int bpp_from_code(__u32 code)
case MEDIA_BUS_FMT_SRGGB12_1X12:
return 12;
case MEDIA_BUS_FMT_UYVY8_1X16:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
case MEDIA_BUS_FMT_RGB565_2X8_LE:
case MEDIA_BUS_FMT_RGB565_2X8_BE:
return 16;
@@ -158,6 +161,8 @@ static u8 data_type_from_code(__u32 code)
case MEDIA_BUS_FMT_SRGGB12_1X12:
return 0x2c;
case MEDIA_BUS_FMT_UYVY8_1X16:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
return 0x1e;
case MEDIA_BUS_FMT_BGR888_1X24:
return 0x24;
--
2.7.4
[PATCH v2 1/3] media: st-mipid02: add support of RGB565
Add support of RGB565 pixel format.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/st-mipid02.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 9369f38..76a9f52 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -61,7 +61,8 @@ static const u32 mipid02_supported_fmt_codes[] = {
MEDIA_BUS_FMT_SGRBG10_1X10, MEDIA_BUS_FMT_SRGGB10_1X10,
MEDIA_BUS_FMT_SBGGR12_1X12, MEDIA_BUS_FMT_SGBRG12_1X12,
MEDIA_BUS_FMT_SGRBG12_1X12, MEDIA_BUS_FMT_SRGGB12_1X12,
- MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_BGR888_1X24
+ MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_BGR888_1X24,
+ MEDIA_BUS_FMT_RGB565_2X8_LE, MEDIA_BUS_FMT_RGB565_2X8_BE,
};
/* regulator supplies */
@@ -128,6 +129,8 @@ static int bpp_from_code(__u32 code)
case MEDIA_BUS_FMT_SRGGB12_1X12:
return 12;
case MEDIA_BUS_FMT_UYVY8_1X16:
+ case MEDIA_BUS_FMT_RGB565_2X8_LE:
+ case MEDIA_BUS_FMT_RGB565_2X8_BE:
return 16;
case MEDIA_BUS_FMT_BGR888_1X24:
return 24;
@@ -158,6 +161,9 @@ static u8 data_type_from_code(__u32 code)
return 0x1e;
case MEDIA_BUS_FMT_BGR888_1X24:
return 0x24;
+ case MEDIA_BUS_FMT_RGB565_2X8_LE:
+ case MEDIA_BUS_FMT_RGB565_2X8_BE:
+ return 0x22;
default:
return 0;
}
--
2.7.4
[PATCH 3/3] media: st-mipid02: add support of JPEG
Add support of JPEG pixel format.
This requires auto detection of data type from CSI-2 stream.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/st-mipid02.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 35946ad..cd275d7 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -64,6 +64,7 @@ static const u32 mipid02_supported_fmt_codes[] = {
MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_BGR888_1X24,
MEDIA_BUS_FMT_RGB565_2X8_LE, MEDIA_BUS_FMT_RGB565_2X8_BE,
MEDIA_BUS_FMT_YUYV8_2X8, MEDIA_BUS_FMT_UYVY8_2X8,
+ MEDIA_BUS_FMT_JPEG_1X8
};
/* regulator supplies */
@@ -101,6 +102,7 @@ struct mipid02_dev {
u8 data_lane1_reg1;
u8 mode_reg1;
u8 mode_reg2;
+ u8 data_selection_ctrl;
u8 data_id_rreg;
u8 pix_width_ctrl;
u8 pix_width_ctrl_emb;
@@ -134,6 +136,7 @@ static int bpp_from_code(__u32 code)
case MEDIA_BUS_FMT_UYVY8_2X8:
case MEDIA_BUS_FMT_RGB565_2X8_LE:
case MEDIA_BUS_FMT_RGB565_2X8_BE:
+ case MEDIA_BUS_FMT_JPEG_1X8:
return 16;
case MEDIA_BUS_FMT_BGR888_1X24:
return 24;
@@ -463,6 +466,7 @@ static int mipid02_configure_from_tx(struct mipid02_dev
*bridge)
{
struct v4l2_fwnode_endpoint *ep = >tx;
+ bridge->r.data_selection_ctrl = SELECTION_MANUAL_WIDTH;
bridge->r.pix_width_ctrl = ep->bus.parallel.bus_width;
bridge->r.pix_width_ctrl_emb = ep->bus.parallel.bus_width;
if (ep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
@@ -478,10 +482,15 @@ static int mipid02_configure_from_code(struct mipid02_dev
*bridge)
u8 data_type;
bridge->r.data_id_rreg = 0;
- data_type = data_type_from_code(bridge->fmt.code);
- if (!data_type)
- return -EINVAL;
- bridge->r.data_id_rreg = data_type;
+
+ if (bridge->fmt.code != MEDIA_BUS_FMT_JPEG_1X8) {
+ bridge->r.data_selection_ctrl |= SELECTION_MANUAL_DATA;
+
+ data_type = data_type_from_code(bridge->fmt.code);
+ if (!data_type)
+ return -EINVAL;
+ bridge->r.data_id_rreg = data_type;
+ }
return 0;
}
@@ -565,7 +574,7 @@ static int mipid02_stream_enable(struct mipid02_dev *bridge)
if (ret)
goto error;
ret = mipid02_write_reg(bridge, MIPID02_DATA_SELECTION_CTRL,
- SELECTION_MANUAL_DATA | SELECTION_MANUAL_WIDTH);
+ bridge->r.data_selection_ctrl);
if (ret)
goto error;
ret = mipid02_write_reg(bridge, MIPID02_PIX_WIDTH_CTRL,
--
2.7.4
[PATCH 1/3] media: st-mipid02: add support of RGB565
Add support of RGB565 pixel format.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/st-mipid02.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 9369f38..76a9f52 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -61,7 +61,8 @@ static const u32 mipid02_supported_fmt_codes[] = {
MEDIA_BUS_FMT_SGRBG10_1X10, MEDIA_BUS_FMT_SRGGB10_1X10,
MEDIA_BUS_FMT_SBGGR12_1X12, MEDIA_BUS_FMT_SGBRG12_1X12,
MEDIA_BUS_FMT_SGRBG12_1X12, MEDIA_BUS_FMT_SRGGB12_1X12,
- MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_BGR888_1X24
+ MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_BGR888_1X24,
+ MEDIA_BUS_FMT_RGB565_2X8_LE, MEDIA_BUS_FMT_RGB565_2X8_BE,
};
/* regulator supplies */
@@ -128,6 +129,8 @@ static int bpp_from_code(__u32 code)
case MEDIA_BUS_FMT_SRGGB12_1X12:
return 12;
case MEDIA_BUS_FMT_UYVY8_1X16:
+ case MEDIA_BUS_FMT_RGB565_2X8_LE:
+ case MEDIA_BUS_FMT_RGB565_2X8_BE:
return 16;
case MEDIA_BUS_FMT_BGR888_1X24:
return 24;
@@ -158,6 +161,9 @@ static u8 data_type_from_code(__u32 code)
return 0x1e;
case MEDIA_BUS_FMT_BGR888_1X24:
return 0x24;
+ case MEDIA_BUS_FMT_RGB565_2X8_LE:
+ case MEDIA_BUS_FMT_RGB565_2X8_BE:
+ return 0x22;
default:
return 0;
}
--
2.7.4
[PATCH 2/3] media: st-mipid02: add support of YUYV8 and UYVY8
Add support of YUYV8 and UYVY8 pixel formats.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/st-mipid02.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 76a9f52..35946ad 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -63,6 +63,7 @@ static const u32 mipid02_supported_fmt_codes[] = {
MEDIA_BUS_FMT_SGRBG12_1X12, MEDIA_BUS_FMT_SRGGB12_1X12,
MEDIA_BUS_FMT_UYVY8_1X16, MEDIA_BUS_FMT_BGR888_1X24,
MEDIA_BUS_FMT_RGB565_2X8_LE, MEDIA_BUS_FMT_RGB565_2X8_BE,
+ MEDIA_BUS_FMT_YUYV8_2X8, MEDIA_BUS_FMT_UYVY8_2X8,
};
/* regulator supplies */
@@ -129,6 +130,8 @@ static int bpp_from_code(__u32 code)
case MEDIA_BUS_FMT_SRGGB12_1X12:
return 12;
case MEDIA_BUS_FMT_UYVY8_1X16:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
case MEDIA_BUS_FMT_RGB565_2X8_LE:
case MEDIA_BUS_FMT_RGB565_2X8_BE:
return 16;
@@ -158,6 +161,8 @@ static u8 data_type_from_code(__u32 code)
case MEDIA_BUS_FMT_SRGGB12_1X12:
return 0x2c;
case MEDIA_BUS_FMT_UYVY8_1X16:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
return 0x1e;
case MEDIA_BUS_FMT_BGR888_1X24:
return 0x24;
--
2.7.4
[PATCH 0/3] Add support of RGB565, YUV and JPEG to MIPID02 bridge
Add support of RGB565, YUV and JPEG to MIPID02 bridge. Hugues Fruchet (3): media: st-mipid02: add support of RGB565 media: st-mipid02: add support of YUYV8 and UYVY8 media: st-mipid02: add support of JPEG drivers/media/i2c/st-mipid02.c | 32 ++-- 1 file changed, 26 insertions(+), 6 deletions(-) -- 2.7.4
[PATCH v2 0/3] 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 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 (3): media: stm32-dcmi: improve sensor subdev naming 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 | 317 +- 2 files changed, 266 insertions(+), 53 deletions(-) -- 2.7.4
[PATCH v2 3/3] media: stm32-dcmi: add support of several sub-devices
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
---
drivers/media/platform/stm32/stm32-dcmi.c | 207 +++---
1 file changed, 189 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 3a69783..144912f 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -173,6 +173,7 @@ struct stm32_dcmi {
struct media_device mdev;
struct media_padvid_cap_pad;
+ struct media_pipeline pipeline;
};
static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier
*n)
@@ -584,6 +585,135 @@ static void dcmi_buf_queue(struct vb2_buffer *vb)
spin_unlock_irq(>irqlock);
}
+static struct media_entity *dcmi_find_sensor(struct stm32_dcmi *dcmi)
+{
+ struct media_entity *entity = >vdev->entity;
+ struct v4l2_subdev *subdev;
+ struct media_pad *pad;
+
+ /* Walk searching for entity having no sink */
+ while (1) {
+ pad = >pads[0];
+
+ subdev = media_entity_to_v4l2_subdev(entity);
+
+ 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 = >sensor->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 = >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, );
+ 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 = >vdev->entity;
+ struct v4l2_subdev *subdev;
+ struct media_pad *pad;
+ int ret;
+
+ /* Start/stop all entities within pipeline */
+ while (1) {
+ pad = >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\
[PATCH v2 2/3] media: stm32-dcmi: add media controller support
Add media controller support to dcmi.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/Kconfig| 2 +-
drivers/media/platform/stm32/stm32-dcmi.c | 83 +++
2 files changed, 63 insertions(+), 22 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 7a4d559..3a69783 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -170,6 +170,9 @@ struct stm32_dcmi {
/* Ensure DMA operations atomicity */
struct mutexdma_lock;
+
+ struct media_device mdev;
+ struct media_padvid_cap_pad;
};
static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier
*n)
@@ -1545,21 +1548,12 @@ static int dcmi_graph_notify_complete(struct
v4l2_async_notifier *notifier)
dev_err(dcmi->dev, "Could not get sensor bounds\n");
return ret;
}
-
ret = dcmi_set_default_fmt(dcmi);
if (ret) {
dev_err(dcmi->dev, "Could not set default format\n");
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;
}
@@ -1648,6 +1642,12 @@ static int dcmi_graph_init(struct stm32_dcmi *dcmi)
return 0;
}
+static void dcmi_graph_deinit(struct stm32_dcmi *dcmi)
+{
+ v4l2_async_notifier_unregister(>notifier);
+ v4l2_async_notifier_cleanup(>notifier);
+}
+
static int dcmi_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -1752,10 +1752,27 @@ static int dcmi_probe(struct platform_device *pdev)
q = >queue;
+ dcmi->v4l2_dev.mdev = >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 = >dev;
+ media_device_init(>mdev);
+
+ /* Register the media device */
+ ret = media_device_register(>mdev);
+ if (ret) {
+ dev_err(dcmi->dev, "Failed to register media device (%d)\n",
+ ret);
+ goto err_media_device_cleanup;
+ }
+
/* Initialize the top-level structure */
ret = v4l2_device_register(>dev, >v4l2_dev);
if (ret)
- goto err_dma_release;
+ goto err_media_device_unregister;
dcmi->vdev = video_device_alloc();
if (!dcmi->vdev) {
@@ -1775,6 +1792,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(>vdev->entity,
+1, >vid_cap_pad);
+ if (ret) {
+ dev_err(dcmi->dev, "Failed to init media entity pad\n");
+ goto err_device_unregister;
+ }
+ 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;
@@ -1790,18 +1826,18 @@ static int dcmi_probe(struct platform_device *pdev)
ret = vb2_queue_init(q);
if (ret < 0) {
dev_err(>dev, "Failed to initialize vb2 queue\n");
- goto err_device_release;
+ goto err_media_entity_cleanup;
}
[PATCH v2 1/3] media: stm32-dcmi: improve sensor subdev naming
Add a new "sensor" field to dcmi struct instead of
reusing entity->subdev to address sensor subdev.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 37 ---
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index b9dad0a..7a4d559 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -151,6 +151,7 @@ struct stm32_dcmi {
unsigned intnum_of_sd_framesizes;
struct dcmi_framesize sd_framesize;
struct v4l2_rectsd_bounds;
+ struct v4l2_subdev *sensor;
/* Protect this data structure */
struct mutexlock;
@@ -595,7 +596,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->sensor, video, s_stream, 1);
if (ret && ret != -ENOIOCTLCMD) {
dev_err(dcmi->dev, "%s: Failed to start streaming, subdev
streamon error",
__func__);
@@ -685,7 +686,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->sensor, video, s_stream, 0);
err_pm_put:
pm_runtime_put(dcmi->dev);
@@ -713,7 +714,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->sensor, 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 +858,7 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct
v4l2_format *f,
}
v4l2_fill_mbus_format(, pix, sd_fmt->mbus_code);
- ret = v4l2_subdev_call(dcmi->entity.subdev, pad, set_fmt,
+ ret = v4l2_subdev_call(dcmi->sensor, pad, set_fmt,
_cfg, );
if (ret < 0)
return ret;
@@ -934,7 +935,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->sensor, pad,
set_fmt, NULL, );
if (ret < 0)
return ret;
@@ -991,7 +992,7 @@ static int dcmi_get_sensor_format(struct stm32_dcmi *dcmi,
};
int ret;
- ret = v4l2_subdev_call(dcmi->entity.subdev, pad, get_fmt, NULL, );
+ ret = v4l2_subdev_call(dcmi->sensor, pad, get_fmt, NULL, );
if (ret)
return ret;
@@ -1020,7 +1021,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
}
v4l2_fill_mbus_format(, pix, sd_fmt->mbus_code);
- ret = v4l2_subdev_call(dcmi->entity.subdev, pad, set_fmt,
+ ret = v4l2_subdev_call(dcmi->sensor, pad, set_fmt,
_cfg, );
if (ret < 0)
return ret;
@@ -1043,7 +1044,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->sensor, pad, get_selection,
NULL, );
if (!ret)
*r = bounds.r;
@@ -1224,7 +1225,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->sensor, pad, enum_frame_size,
NULL, );
if (ret)
return ret;
@@ -1241,7 +1242,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->sensor, p);
}
static int dcmi_s_parm(struct file *file, void *priv,
@@ -1249,7 +1250,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->sensor, p);
}
sta
Re: [PATCH 10/17] delta: Fix buffer overrun in delta_ipc_open
Hi Andi, So do both, memset then strscpy: + memset(msg.name, 0, sizeof(msg.name)); + if (strscpy(msg.name, name, sizeof(msg.name)) <= 0) + goto err; BR, Hugues. On 4/1/19 6:54 PM, Andi Kleen wrote: > On Mon, Apr 01, 2019 at 01:37:56PM +, Hugues FRUCHET wrote: >> Hi Andi, >> >> We have already discussed about that here: >> https://lore.kernel.org/patchwork/patch/866406/ >> >> Now that strscpy is largely deployed within kernel, could you retest >> with the change I suggested ? > > strscpy is not the correct fix because it leaks uninitialized memory > to the receiver. You need the memset. > > -Andi >
Re: [PATCH 10/17] delta: Fix buffer overrun in delta_ipc_open
Hi Andi, We have already discussed about that here: https://lore.kernel.org/patchwork/patch/866406/ Now that strscpy is largely deployed within kernel, could you retest with the change I suggested ? Best regards, Hugues. On 3/21/19 11:00 PM, Andi Kleen wrote: > From: Andi Kleen > > delta_ipc_open is always called with a single constant string > as name, but it uses a longer memcpy to copy the string to > a different structure. The memcpy would read outside the bounds > of the string, potentially accessing unmapped memory. > > Just use strcpy instead after clearing the area. > > This fixes a build error with LTO, which can detect this. > > Cc: hugues.fruc...@st.com > Cc: mche...@s-opensource.com > Fixes: 91c83f395fbe [media] st-delta: rpmsg ipc support > Signed-off-by: Andi Kleen > --- > drivers/media/platform/sti/delta/delta-ipc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/sti/delta/delta-ipc.c > b/drivers/media/platform/sti/delta/delta-ipc.c > index a4603d573c34..bd1bbbeedec3 100644 > --- a/drivers/media/platform/sti/delta/delta-ipc.c > +++ b/drivers/media/platform/sti/delta/delta-ipc.c > @@ -175,8 +175,8 @@ int delta_ipc_open(struct delta_ctx *pctx, const char > *name, > msg.ipc_buf_size = ipc_buf_size; > msg.ipc_buf_paddr = ctx->ipc_buf->paddr; > > - memcpy(msg.name, name, sizeof(msg.name)); > - msg.name[sizeof(msg.name) - 1] = 0; > + memset(msg.name, 0, sizeof(msg.name)); > + strcpy(msg.name, name); > > msg.param_size = param->size; > memcpy(ctx->ipc_buf->vaddr, param->data, msg.param_size); >
[PATCH 0/2] Refine DCMI error messages
Cleanup and refine some DCMI error messages. Hugues Fruchet (2): media: stm32-dcmi: fix error messages media: stm32-dcmi: do not emit error trace in case of few overrun drivers/media/platform/stm32/stm32-dcmi.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) -- 2.7.4
[PATCH 2/2] media: stm32-dcmi: do not emit error trace in case of few overrun
Report overrun error only when it exceeds a given threshold.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 693415c..b63d57c 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -97,6 +97,8 @@ enum state {
#define TIMEOUT_MS 1000
+#define OVERRUN_ERROR_THRESHOLD3
+
struct dcmi_graph_entity {
struct device_node *node;
@@ -446,11 +448,13 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
spin_lock_irq(>irqlock);
- if ((dcmi->misr & IT_OVR) || (dcmi->misr & IT_ERR)) {
- dcmi->errors_count++;
- if (dcmi->misr & IT_OVR)
- dcmi->overrun_count++;
+ if (dcmi->misr & IT_OVR) {
+ dcmi->overrun_count++;
+ if (dcmi->overrun_count > OVERRUN_ERROR_THRESHOLD)
+ dcmi->errors_count++;
}
+ if (dcmi->misr & IT_ERR)
+ dcmi->errors_count++;
if (dcmi->sd_format->fourcc == V4L2_PIX_FMT_JPEG &&
dcmi->misr & IT_FRAME) {
--
2.7.4
Re: [PATCH] media: uvcvideo: Read support
Hi Kieran, Laurent,
On 3/4/19 4:44 PM, Laurent Pinchart wrote:
> Hello,
>
> On Mon, Mar 04, 2019 at 03:36:32PM +, Kieran Bingham wrote:
>> On 04/03/2019 12:35, Hugues Fruchet wrote:
>>> Add support of read() call from userspace by implementing
>>> uvc_v4l2_read() with vb2_read() helper.
>>
>> Just thinking out loud,
>>
>> This opens up UVC devices to read raw full frame images through this
>> interface as well.
>>
>> Due to the UVC protocol, there is /already/ a full memcpy to get these
>> images out of the URB packets, so using a read() interface would be
>> another full frame copy.
>>
>> I can perhaps see the usecase for reading compressed data through this
>> interface - but full frames don't seem appropriate. (not impossible of
>> course, just is it reasonable?)
>>
>> If this is to be enabled, should it be enabled for compressed formats
>> only? or would that complicate matters?
>
> I've repeatedly refused read() support in uvcvideo for this reason, and
> also because read() doesn't carry framing information very well. It's
> just not a good API for capturing video frames from a webcam, and so far
> I haven't heard a compeling reason why it should be enabled. I thus
> haven't changed my mind :-)
>
For sure read() is not optimal, but is very common and pretty simple to
use from userspace as explained in the cover letter with some examples.
Moreover, read() is enabled by many (all ?) camera interfaces:
drivers/media/platform/pxa_camera.c:.read = vb2_fop_read,
drivers/media/platform/atmel/atmel-isi.c: .read = vb2_fop_read,
drivers/media/platform/rcar-vin/rcar-v4l2.c:.read = vb2_fop_read,
drivers/media/platform/stm32/stm32-dcmi.c: .read = vb2_fop_read,
drivers/media/platform/vivid/vivid-core.c: .read = vb2_fop_read,
drivers/media/platform/vimc/vimc-capture.c: .read = vb2_fop_read,
drivers/media/platform/marvell-ccic/mcam-core.c:.read = vb2_fop_read,
drivers/media/platform/qcom/camss/camss-video.c:.read = vb2_fop_read,
on my setup, this leads to have /dev/video0 (mapped on DCMI camera
interface) supporting JPEG streaming through read() while /dev/video1
(mapped on Hercule USB camera) fails with "invalid argument" error...
>>> Signed-off-by: Hugues Fruchet
>>> ---
>>> drivers/media/usb/uvc/uvc_queue.c | 15 ++-
>>> drivers/media/usb/uvc/uvc_v4l2.c | 11 ---
>>> drivers/media/usb/uvc/uvcvideo.h | 2 ++
>>> 3 files changed, 24 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/media/usb/uvc/uvc_queue.c
>>> b/drivers/media/usb/uvc/uvc_queue.c
>>> index 682698e..0c8a0a8 100644
>>> --- a/drivers/media/usb/uvc/uvc_queue.c
>>> +++ b/drivers/media/usb/uvc/uvc_queue.c
>>> @@ -227,7 +227,7 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum
>>> v4l2_buf_type type,
>>> int ret;
>>>
>>> queue->queue.type = type;
>>> - queue->queue.io_modes = VB2_MMAP | VB2_USERPTR;
>>> + queue->queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ;
>>> queue->queue.drv_priv = queue;
>>> queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
>>> queue->queue.mem_ops = _vmalloc_memops;
>>> @@ -361,6 +361,19 @@ int uvc_queue_streamoff(struct uvc_video_queue *queue,
>>> enum v4l2_buf_type type)
>>> return ret;
>>> }
>>>
>>> +ssize_t uvc_queue_read(struct uvc_video_queue *queue, struct file *file,
>>> + char __user *buf, size_t count, loff_t *ppos)
>>> +{
>>> + ssize_t ret;
>>> +
>>> + mutex_lock(>mutex);
>>> + ret = vb2_read(>queue, buf, count, ppos,
>>> + file->f_flags & O_NONBLOCK);
>>> + mutex_unlock(>mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> int uvc_queue_mmap(struct uvc_video_queue *queue, struct vm_area_struct
>>> *vma)
>>> {
>>> return vb2_mmap(>queue, vma);
>>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
>>> b/drivers/media/usb/uvc/uvc_v4l2.c
>>> index 84be596..3866832 100644
>>> --- a/drivers/media/usb/uvc/uvc_v4l2.c
>>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>>> @@ -594,7 +594,8 @@ static int uvc_ioctl_querycap(struct file *file, void
>>> *fh,
>>> strscpy(cap->driver, "uvcvideo", sizeof(cap->driver));
>>> strscpy(cap->card, vdev->name, sizeof(cap->card));
>>> usb_make_path(stream->dev->udev
[PATCH] media: uvcvideo: Read support
Add support of read() call from userspace by implementing
uvc_v4l2_read() with vb2_read() helper.
Signed-off-by: Hugues Fruchet
---
drivers/media/usb/uvc/uvc_queue.c | 15 ++-
drivers/media/usb/uvc/uvc_v4l2.c | 11 ---
drivers/media/usb/uvc/uvcvideo.h | 2 ++
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index 682698e..0c8a0a8 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -227,7 +227,7 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum
v4l2_buf_type type,
int ret;
queue->queue.type = type;
- queue->queue.io_modes = VB2_MMAP | VB2_USERPTR;
+ queue->queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ;
queue->queue.drv_priv = queue;
queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
queue->queue.mem_ops = _vmalloc_memops;
@@ -361,6 +361,19 @@ int uvc_queue_streamoff(struct uvc_video_queue *queue,
enum v4l2_buf_type type)
return ret;
}
+ssize_t uvc_queue_read(struct uvc_video_queue *queue, struct file *file,
+ char __user *buf, size_t count, loff_t *ppos)
+{
+ ssize_t ret;
+
+ mutex_lock(>mutex);
+ ret = vb2_read(>queue, buf, count, ppos,
+ file->f_flags & O_NONBLOCK);
+ mutex_unlock(>mutex);
+
+ return ret;
+}
+
int uvc_queue_mmap(struct uvc_video_queue *queue, struct vm_area_struct *vma)
{
return vb2_mmap(>queue, vma);
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 84be596..3866832 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -594,7 +594,8 @@ static int uvc_ioctl_querycap(struct file *file, void *fh,
strscpy(cap->driver, "uvcvideo", sizeof(cap->driver));
strscpy(cap->card, vdev->name, sizeof(cap->card));
usb_make_path(stream->dev->udev, cap->bus_info, sizeof(cap->bus_info));
- cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING
+ cap->capabilities = V4L2_CAP_DEVICE_CAPS | V4L2_CAP_STREAMING |
+ V4L2_CAP_READWRITE
| chain->caps;
return 0;
@@ -1434,8 +1435,12 @@ static long uvc_v4l2_compat_ioctl32(struct file *file,
static ssize_t uvc_v4l2_read(struct file *file, char __user *data,
size_t count, loff_t *ppos)
{
- uvc_trace(UVC_TRACE_CALLS, "uvc_v4l2_read: not implemented.\n");
- return -EINVAL;
+ struct uvc_fh *handle = file->private_data;
+ struct uvc_streaming *stream = handle->stream;
+
+ uvc_trace(UVC_TRACE_CALLS, "uvc_v4l2_read\n");
+
+ return uvc_queue_read(>queue, file, data, count, ppos);
}
static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma)
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index c7c1baa..5d0515c 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -766,6 +766,8 @@ struct uvc_buffer *uvc_queue_next_buffer(struct
uvc_video_queue *queue,
struct uvc_buffer *buf);
struct uvc_buffer *uvc_queue_get_current_buffer(struct uvc_video_queue *queue);
void uvc_queue_buffer_release(struct uvc_buffer *buf);
+ssize_t uvc_queue_read(struct uvc_video_queue *queue, struct file *file,
+ char __user *buf, size_t count, loff_t *ppos);
int uvc_queue_mmap(struct uvc_video_queue *queue,
struct vm_area_struct *vma);
__poll_t uvc_queue_poll(struct uvc_video_queue *queue, struct file *file,
--
2.7.4
[PATCH] Support of /dev/video read with USB camera devices
Currently, read() call from /dev/video entry of an USB camera is returning invalid argument error. This is quite common for userspace application to read compressed data such as JPEG from /dev/video then redirect those data to a multimedia player or any other compressed format consumer, see [1]. There was a tentative of implementation in the past [2] but it was prior to switch on vb2 helpers usage inside uvc. Now that vb2 is in place, we can implement read support using vb2_read() helper. Tested with Logitech HD Webcam C525 using v4l2-ctl to configure camera then GStreamer player to play stream: $> v4l2-ctl -d /dev/video0 --set-fmt-video=width=640,height=480,pixelformat=MJPG $> gst-play-1.0 /dev/video0 [1] http://credentiality2.blogspot.com/2010/04/v4l2-example.html?_sm_au_=ikcQHNZZFn2Rqft5 https://stackoverrun.com/fr/q/4215615 https://stackoverflow.com/questions/36297390/read-function-for-webcam-device-in-v4l2-fails-with-invalid-argument https://stackoverflow.com/questions/31058571/reading-camera-input-from-dev-video0-in-python-or-c [2] https://www.mail-archive.com/linux-uvc-devel@lists.berlios.de/msg01258.html Hugues Fruchet (1): media: uvcvideo: Read support drivers/media/usb/uvc/uvc_queue.c | 15 ++- drivers/media/usb/uvc/uvc_v4l2.c | 11 --- drivers/media/usb/uvc/uvcvideo.h | 2 ++ 3 files changed, 24 insertions(+), 4 deletions(-) -- 2.7.4
[PATCH] media: stm32-dcmi: fix DMA corruption when stopping streaming
Avoid call of dmaengine_terminate_all() between
dmaengine_prep_slave_single() and dmaengine_submit() by locking
the whole DMA submission sequence.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index f44d8a7..a611a21 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -164,6 +164,9 @@ struct stm32_dcmi {
int errors_count;
int overrun_count;
int buffers_count;
+
+ /* Ensure DMA operations atomicity */
+ struct mutexdma_lock;
};
static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier
*n)
@@ -314,6 +317,13 @@ static int dcmi_start_dma(struct stm32_dcmi *dcmi,
return ret;
}
+ /*
+* Avoid call of dmaengine_terminate_all() between
+* dmaengine_prep_slave_single() and dmaengine_submit()
+* by locking the whole DMA submission sequence
+*/
+ mutex_lock(>dma_lock);
+
/* Prepare a DMA transaction */
desc = dmaengine_prep_slave_single(dcmi->dma_chan, buf->paddr,
buf->size,
@@ -322,6 +332,7 @@ static int dcmi_start_dma(struct stm32_dcmi *dcmi,
if (!desc) {
dev_err(dcmi->dev, "%s: DMA dmaengine_prep_slave_single failed
for buffer phy=%pad size=%zu\n",
__func__, >paddr, buf->size);
+ mutex_unlock(>dma_lock);
return -EINVAL;
}
@@ -333,9 +344,12 @@ static int dcmi_start_dma(struct stm32_dcmi *dcmi,
dcmi->dma_cookie = dmaengine_submit(desc);
if (dma_submit_error(dcmi->dma_cookie)) {
dev_err(dcmi->dev, "%s: DMA submission failed\n", __func__);
+ mutex_unlock(>dma_lock);
return -ENXIO;
}
+ mutex_unlock(>dma_lock);
+
dma_async_issue_pending(dcmi->dma_chan);
return 0;
@@ -720,7 +734,9 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
spin_unlock_irq(>irqlock);
/* Stop all pending DMA operations */
+ mutex_lock(>dma_lock);
dmaengine_terminate_all(dcmi->dma_chan);
+ mutex_unlock(>dma_lock);
pm_runtime_put(dcmi->dev);
@@ -1711,6 +1727,7 @@ static int dcmi_probe(struct platform_device *pdev)
spin_lock_init(>irqlock);
mutex_init(>lock);
+ mutex_init(>dma_lock);
init_completion(>complete);
INIT_LIST_HEAD(>buffers);
--
2.7.4
[PATCH] media: stm32-dcmi: fix check of pm_runtime_get_sync return value
Start streaming was sometimes failing because of pm_runtime_get_sync()
non-0 return value. In fact return value was not an error but a
positive value (1), indicating that PM was already enabled.
Fix this by going to error path only with negative return value.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 6732874..f44d8a7 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -570,9 +570,9 @@ static int dcmi_start_streaming(struct vb2_queue *vq,
unsigned int count)
int ret;
ret = pm_runtime_get_sync(dcmi->dev);
- if (ret) {
- dev_err(dcmi->dev, "%s: Failed to start streaming, cannot get
sync\n",
- __func__);
+ if (ret < 0) {
+ dev_err(dcmi->dev, "%s: Failed to start streaming, cannot get
sync (%d)\n",
+ __func__, ret);
goto err_release_buffers;
}
--
2.7.4
[PATCH 2/4] ARM: dts: stm32: add DCMI camera interface support on stm32mp157c
Add DCMI camera interface support on stm32mp157c.
Signed-off-by: Hugues Fruchet
---
arch/arm/boot/dts/stm32mp157c.dtsi | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi
b/arch/arm/boot/dts/stm32mp157c.dtsi
index 10bf338..1ff331c 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -886,6 +886,18 @@
status = "disabled";
};
+ dcmi: dcmi@4c006000 {
+ compatible = "st,stm32-dcmi";
+ reg = <0x4c006000 0x400>;
+ interrupts = ;
+ resets = < CAMITF_R>;
+ clocks = < DCMI>;
+ clock-names = "mclk";
+ dmas = < 75 0x400 0x0d>;
+ dma-names = "tx";
+ status = "disabled";
+ };
+
rcc: rcc@5000 {
compatible = "st,stm32mp1-rcc", "syscon";
reg = <0x5000 0x1000>;
--
2.7.4
[PATCH 3/4] ARM: dts: stm32: add DCMI pins to stm32mp157c
Add DCMI pins to stm32mp157c.
Signed-off-by: Hugues Fruchet
---
arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 41 +++
1 file changed, 41 insertions(+)
diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
index 6b3a9c6..b299afc 100644
--- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
@@ -414,6 +414,47 @@
bias-disable;
};
};
+
+ dcmi_pins_a: dcmi-0 {
+ pins {
+ pinmux = ,/* DCMI_HSYNC */
+,/* DCMI_VSYNC */
+,/* DCMI_PIXCLK */
+,/* DCMI_D0 */
+,/* DCMI_D1 */
+,/* DCMI_D2 */
+,/* DCMI_D3 */
+,/* DCMI_D4 */
+,/* DCMI_D5 */
+,/* DCMI_D6 */
+,/* DCMI_D7 */
+,/* DCMI_D8 */
+,/* DCMI_D9 */
+,/* DCMI_D10 */
+;/* DCMI_D11 */
+ bias-disable;
+ };
+ };
+
+ dcmi_sleep_pins_a: dcmi-sleep-0 {
+ pins {
+ pinmux = ,/* DCMI_HSYNC */
+,/* DCMI_VSYNC */
+,/* DCMI_PIXCLK */
+,/* DCMI_D0 */
+,/* DCMI_D1 */
+,/* DCMI_D2 */
+,/* DCMI_D3 */
+,/* DCMI_D4 */
+,/* DCMI_D5 */
+,/* DCMI_D6 */
+,/* DCMI_D7 */
+,/* DCMI_D8 */
+,/* DCMI_D9 */
+,/* DCMI_D10 */
+;/* DCMI_D11 */
+ };
+ };
};
pinctrl_z: pin-controller-z@54004000 {
--
2.7.4
[PATCH 1/4] ARM: dts: stm32: add 2v8 fixed regulator for stm32mp157c-ed1 board
Add 2v8 fixed regulator and enable it.
Signed-off-by: Hugues Fruchet
---
arch/arm/boot/dts/stm32mp157c-ed1.dts | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts
b/arch/arm/boot/dts/stm32mp157c-ed1.dts
index 10dc7c0..4b5bcfd 100644
--- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
@@ -57,6 +57,14 @@
regulator-boot-on;
};
+ v2v8: ldo2 {
+ compatible = "regulator-fixed";
+ regulator-name = "v2v8";
+ regulator-min-microvolt = <280>;
+ regulator-max-microvolt = <280>;
+ regulator-always-on;
+ };
+
sd_switch: regulator-sd_switch {
compatible = "regulator-gpio";
regulator-name = "sd_switch";
--
2.7.4
[PATCH 0/4] Enable stm32mp1 camera support
This patchset serie enables camera of stm32mp157c evaluation board. To do so, both stm32mp157c DCMI camera interface driver and OV5640 camera module driver are enabled in devicetree. DCMI camera interface driver is enabled by default in multi_v7 configuration but OV5640 driver remains to be selected at kernel configuration time. This patchset depends on STMicroelectronics Multi-Function eXpander (STMFX) support [1]. [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1944108.html Hugues Fruchet (4): ARM: dts: stm32: add 2v8 fixed regulator for stm32mp157c-ed1 board ARM: dts: stm32: add DCMI camera interface support on stm32mp157c ARM: dts: stm32: add DCMI pins to stm32mp157c ARM: dts: stm32: enable OV5640 camera on stm32mp157c-ev1 board arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 41 ++ arch/arm/boot/dts/stm32mp157c-ed1.dts | 8 + arch/arm/boot/dts/stm32mp157c-ev1.dts | 56 +++ arch/arm/boot/dts/stm32mp157c.dtsi| 12 +++ 4 files changed, 117 insertions(+) -- 2.7.4
[PATCH 4/4] ARM: dts: stm32: enable OV5640 camera on stm32mp157c-ev1 board
Enable OV5640 camera sensor driver of MB1379A extension
board connected on CN7 connector of stm32mp157c-ev1 board:
bus-width is set to 8, data-shift is set to 2 (lines 9:2 are used),
hsync-active is set to 0 for horizontal synchro line active low,
vsync-active is set to 0 for vertical synchro line active low and
pclk-sample is set to 1 for pixel clock polarity sampling data
on rising edge of the pixel clock signal.
Signed-off-by: Hugues Fruchet
---
arch/arm/boot/dts/stm32mp157c-ev1.dts | 56 +++
1 file changed, 56 insertions(+)
diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts
b/arch/arm/boot/dts/stm32mp157c-ev1.dts
index 009f9d6..8fb1688 100644
--- a/arch/arm/boot/dts/stm32mp157c-ev1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts
@@ -65,6 +65,14 @@
default-on;
status = "okay";
};
+
+ clocks {
+ clk_ext_camera: clk-ext-camera {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <2400>;
+ };
+ };
};
{
@@ -73,6 +81,23 @@
status = "okay";
};
+ {
+ status = "okay";
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <_pins_a>;
+ pinctrl-1 = <_sleep_pins_a>;
+
+ port {
+ dcmi_0: endpoint {
+ remote-endpoint = <_0>;
+ bus-width = <8>;
+ hsync-active = <0>;
+ vsync-active = <0>;
+ pclk-sample = <1>;
+ };
+ };
+};
+
{
#address-cells = <1>;
#size-cells = <0>;
@@ -138,6 +163,31 @@
i2c-scl-falling-time-ns = <20>;
status = "okay";
+ ov5640: camera@3c {
+ compatible = "ovti,ov5640";
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ reg = <0x3c>;
+ clocks = <_ext_camera>;
+ clock-names = "xclk";
+ DOVDD-supply = <>;
+ powerdown-gpios = <_pinctrl 18 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <_pinctrl 19 GPIO_ACTIVE_LOW>;
+ rotation = <180>;
+ status = "okay";
+
+ port {
+ ov5640_0: endpoint {
+ remote-endpoint = <_0>;
+ bus-width = <8>;
+ data-shift = <2>; /* lines 9:2 are used */
+ hsync-active = <0>;
+ vsync-active = <0>;
+ pclk-sample = <1>;
+ };
+ };
+ };
+
stmfx: stmfx@42 {
compatible = "st,stmfx-0300";
reg = <0x42>;
@@ -157,6 +207,12 @@
drive-push-pull;
bias-pull-down;
};
+
+ ov5640_pins: camera {
+ pins = "agpio2", "agpio3"; /* stmfx pins 18 &
19 */
+ drive-push-pull;
+ output-low;
+ };
};
};
};
--
2.7.4
Re: [PATCH 1/2] media: stm32-dcmi: drop unnecessary while(1) loop
Hi Nicholas,
thanks for patch !
On 06/12/2018 07:22 PM, Nicholas Mc Guire wrote:
> The while(1) is effectively useless as all possible paths within it
> return thus there is no way to loop.
>
> Signed-off-by: Nicholas Mc Guire
Acked-by: Hugues Fruchet
> ---
>
> This is not actually fixing any bug - the while(1){ } will not hurt here
> it is though simply unnecessary. Found during code review.
>
> The diff output is not very readable - essentially only the outer
> while(1){ } was removed.
>
> Patch was compile tested with: x86_64_defconfig, MEDIA_SUPPORT=y
> MEDIA_CAMERA_SUPPORT=y, V4L_PLATFORM_DRIVERS=y, OF=y, COMPILE_TEST=y
> CONFIG_VIDEO_STM32_DCMI=y
> (There are a number of sparse warnings - not related to the changes though)
>
> Patch is against 4.17.0 (localversion-next is next-20180608)
>
> drivers/media/platform/stm32/stm32-dcmi.c | 28 +---
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index 2e1933d..70b81d2 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -1605,23 +1605,21 @@ static int dcmi_graph_parse(struct stm32_dcmi *dcmi,
> struct device_node *node)
> struct device_node *ep = NULL;
> struct device_node *remote;
>
> - while (1) {
> - ep = of_graph_get_next_endpoint(node, ep);
> - if (!ep)
> - return -EINVAL;
> -
> - remote = of_graph_get_remote_port_parent(ep);
> - if (!remote) {
> - of_node_put(ep);
> - return -EINVAL;
> - }
> + ep = of_graph_get_next_endpoint(node, ep);
> + if (!ep)
> + return -EINVAL;
>
> - /* Remote node to connect */
> - dcmi->entity.node = remote;
> - dcmi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - dcmi->entity.asd.match.fwnode = of_fwnode_handle(remote);
> - return 0;
> + remote = of_graph_get_remote_port_parent(ep);
> + if (!remote) {
> + of_node_put(ep);
> + return -EINVAL;
> }
> +
> + /* Remote node to connect */
> + dcmi->entity.node = remote;
> + dcmi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> + dcmi->entity.asd.match.fwnode = of_fwnode_handle(remote);
> + return 0;
> }
>
> static int dcmi_graph_init(struct stm32_dcmi *dcmi)
>
BR,
Hugues.
Re: [PATCH 1/2] media: stm32-dcmi: drop unnecessary while(1) loop
Hi Nicholas,
thanks for patch !
On 06/12/2018 07:22 PM, Nicholas Mc Guire wrote:
> The while(1) is effectively useless as all possible paths within it
> return thus there is no way to loop.
>
> Signed-off-by: Nicholas Mc Guire
Acked-by: Hugues Fruchet
> ---
>
> This is not actually fixing any bug - the while(1){ } will not hurt here
> it is though simply unnecessary. Found during code review.
>
> The diff output is not very readable - essentially only the outer
> while(1){ } was removed.
>
> Patch was compile tested with: x86_64_defconfig, MEDIA_SUPPORT=y
> MEDIA_CAMERA_SUPPORT=y, V4L_PLATFORM_DRIVERS=y, OF=y, COMPILE_TEST=y
> CONFIG_VIDEO_STM32_DCMI=y
> (There are a number of sparse warnings - not related to the changes though)
>
> Patch is against 4.17.0 (localversion-next is next-20180608)
>
> drivers/media/platform/stm32/stm32-dcmi.c | 28 +---
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index 2e1933d..70b81d2 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -1605,23 +1605,21 @@ static int dcmi_graph_parse(struct stm32_dcmi *dcmi,
> struct device_node *node)
> struct device_node *ep = NULL;
> struct device_node *remote;
>
> - while (1) {
> - ep = of_graph_get_next_endpoint(node, ep);
> - if (!ep)
> - return -EINVAL;
> -
> - remote = of_graph_get_remote_port_parent(ep);
> - if (!remote) {
> - of_node_put(ep);
> - return -EINVAL;
> - }
> + ep = of_graph_get_next_endpoint(node, ep);
> + if (!ep)
> + return -EINVAL;
>
> - /* Remote node to connect */
> - dcmi->entity.node = remote;
> - dcmi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - dcmi->entity.asd.match.fwnode = of_fwnode_handle(remote);
> - return 0;
> + remote = of_graph_get_remote_port_parent(ep);
> + if (!remote) {
> + of_node_put(ep);
> + return -EINVAL;
> }
> +
> + /* Remote node to connect */
> + dcmi->entity.node = remote;
> + dcmi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> + dcmi->entity.asd.match.fwnode = of_fwnode_handle(remote);
> + return 0;
> }
>
> static int dcmi_graph_init(struct stm32_dcmi *dcmi)
>
BR,
Hugues.
[PATCH] media: stm32-dcmi: revisit stop streaming ops
Do not wait for interrupt completion when stopping streaming,
stopping sensor and disabling interruptions are enough.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 29 +
1 file changed, 1 insertion(+), 28 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 581ded0..f0134a6 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -87,7 +87,6 @@ enum state {
STOPPED = 0,
WAIT_FOR_BUFFER,
RUNNING,
- STOPPING,
};
#define MIN_WIDTH 16U
@@ -432,18 +431,6 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
spin_lock_irq(>irqlock);
- /* Stop capture is required */
- if (dcmi->state == STOPPING) {
- reg_clear(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR);
-
- dcmi->state = STOPPED;
-
- complete(>complete);
-
- spin_unlock_irq(>irqlock);
- return IRQ_HANDLED;
- }
-
if ((dcmi->misr & IT_OVR) || (dcmi->misr & IT_ERR)) {
dcmi->errors_count++;
if (dcmi->misr & IT_OVR)
@@ -701,8 +688,6 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
{
struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
struct dcmi_buf *buf, *node;
- unsigned long time_ms = msecs_to_jiffies(TIMEOUT_MS);
- long timeout;
int ret;
/* Disable stream on the sub device */
@@ -712,13 +697,6 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
__func__, ret);
spin_lock_irq(>irqlock);
- dcmi->state = STOPPING;
- spin_unlock_irq(>irqlock);
-
- timeout = wait_for_completion_interruptible_timeout(>complete,
- time_ms);
-
- spin_lock_irq(>irqlock);
/* Disable interruptions */
reg_clear(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR);
@@ -726,12 +704,6 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
/* Disable DCMI */
reg_clear(dcmi->regs, DCMI_CR, CR_ENABLE);
- if (!timeout) {
- dev_err(dcmi->dev, "%s: Timeout during stop streaming\n",
- __func__);
- dcmi->state = STOPPED;
- }
-
/* Return all queued buffers to vb2 in ERROR state */
list_for_each_entry_safe(buf, node, >buffers, list) {
list_del_init(>list);
@@ -739,6 +711,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
}
dcmi->active = NULL;
+ dcmi->state = STOPPED;
spin_unlock_irq(>irqlock);
--
1.9.1
[PATCH] media: stm32-dcmi: revisit stop streaming ops
Do not wait for interrupt completion when stopping streaming,
stopping sensor and disabling interruptions are enough.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/stm32/stm32-dcmi.c | 29 +
1 file changed, 1 insertion(+), 28 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 581ded0..f0134a6 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -87,7 +87,6 @@ enum state {
STOPPED = 0,
WAIT_FOR_BUFFER,
RUNNING,
- STOPPING,
};
#define MIN_WIDTH 16U
@@ -432,18 +431,6 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
spin_lock_irq(>irqlock);
- /* Stop capture is required */
- if (dcmi->state == STOPPING) {
- reg_clear(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR);
-
- dcmi->state = STOPPED;
-
- complete(>complete);
-
- spin_unlock_irq(>irqlock);
- return IRQ_HANDLED;
- }
-
if ((dcmi->misr & IT_OVR) || (dcmi->misr & IT_ERR)) {
dcmi->errors_count++;
if (dcmi->misr & IT_OVR)
@@ -701,8 +688,6 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
{
struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
struct dcmi_buf *buf, *node;
- unsigned long time_ms = msecs_to_jiffies(TIMEOUT_MS);
- long timeout;
int ret;
/* Disable stream on the sub device */
@@ -712,13 +697,6 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
__func__, ret);
spin_lock_irq(>irqlock);
- dcmi->state = STOPPING;
- spin_unlock_irq(>irqlock);
-
- timeout = wait_for_completion_interruptible_timeout(>complete,
- time_ms);
-
- spin_lock_irq(>irqlock);
/* Disable interruptions */
reg_clear(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR);
@@ -726,12 +704,6 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
/* Disable DCMI */
reg_clear(dcmi->regs, DCMI_CR, CR_ENABLE);
- if (!timeout) {
- dev_err(dcmi->dev, "%s: Timeout during stop streaming\n",
- __func__);
- dcmi->state = STOPPED;
- }
-
/* Return all queued buffers to vb2 in ERROR state */
list_for_each_entry_safe(buf, node, >buffers, list) {
list_del_init(>list);
@@ -739,6 +711,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
}
dcmi->active = NULL;
+ dcmi->state = STOPPED;
spin_unlock_irq(>irqlock);
--
1.9.1
