This series is the v6 series that attempts to support media controller in the
atmel ISC and XISC drivers.
The CSI2DC driver was accepted thus removed from the patch series, together with
other patches.
Important note: this series applies on top of current media_staging tree, as it
relies on previous patches in the series which were accepted.
Thanks to everyone who reviewed my work !
Eugen
Changes in v6:
-> worked a bit on scaler, added try crop and other changes as per Jacopo review
-> worked on isc-base enum_fmt , reworked as per Jacopo review
Changes in v5:
-> removed patch that removed the 'stop' variable as it was still required
-> added two new trivial patches
-> reworked some parts of the scaler and format propagation after discussions with Jacopo
Changes in v4:
-> as reviewed by Hans, added new patch to remove the 'stop' variable and reworked
one patch that was using it
-> as reviewed by Jacopo, reworked some parts of the media controller implementation
Changes in v3:
- change in bindings, small fixes in csi2dc driver and conversion to mc
for the isc-base.
- removed some MAINTAINERS patches and used patterns in MAINTAINERS
Changes in v2:
- integrated many changes suggested by Jacopo in the review of the v1 series.
- add a few new patches
Eugen Hristev (13):
media: atmel: atmel-isc-base: use streaming status when queueing
buffers
media: atmel: atmel-isc-base: replace is_streaming call in
s_fmt_vid_cap
media: atmel: atmel-isc: remove redundant comments
media: atmel: atmel-isc: implement media controller
media: atmel: atmel-sama5d2-isc: fix wrong mask in YUYV format check
media: atmel: atmel-isc-base: use mutex to lock awb workqueue from
streaming
media: atmel: atmel-isc: compact the controller formats list
media: atmel: atmel-isc: change format propagation to subdev into only
verification
media: atmel: atmel-sama7g5-isc: remove stray line
dt-bindings: media: microchip,xisc: add bus-width of 14
ARM: dts: at91: sama7g5: add nodes for video capture
ARM: configs: at91: sama7: add xisc and csi2dc
ARM: multi_v7_defconfig: add atmel video pipeline modules
.../bindings/media/microchip,xisc.yaml | 2 +-
arch/arm/boot/dts/sama7g5.dtsi | 49 ++
arch/arm/configs/multi_v7_defconfig | 3 +
arch/arm/configs/sama7_defconfig | 2 +
drivers/media/platform/atmel/Makefile | 2 +-
drivers/media/platform/atmel/atmel-isc-base.c | 524 ++++++++++--------
.../media/platform/atmel/atmel-isc-scaler.c | 276 +++++++++
drivers/media/platform/atmel/atmel-isc.h | 58 +-
.../media/platform/atmel/atmel-sama5d2-isc.c | 87 +--
.../media/platform/atmel/atmel-sama7g5-isc.c | 93 ++--
10 files changed, 765 insertions(+), 331 deletions(-)
create mode 100644 drivers/media/platform/atmel/atmel-isc-scaler.c
--
2.25.1
Remove stray line from formats struct.
Signed-off-by: Eugen Hristev <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
drivers/media/platform/atmel/atmel-sama7g5-isc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/platform/atmel/atmel-sama7g5-isc.c b/drivers/media/platform/atmel/atmel-sama7g5-isc.c
index a0d60cfdba7b..638af8da2694 100644
--- a/drivers/media/platform/atmel/atmel-sama7g5-isc.c
+++ b/drivers/media/platform/atmel/atmel-sama7g5-isc.c
@@ -225,7 +225,6 @@ static struct isc_format sama7g5_formats_list[] = {
.mbus_code = MEDIA_BUS_FMT_Y10_1X10,
.pfe_cfg0_bps = ISC_PFG_CFG0_BPS_TEN,
},
-
};
static void isc_sama7g5_config_csc(struct isc_device *isc)
--
2.25.1
Add drivers for the atmel video capture pipeline: atmel isc, xisc and
microchip csi2dc.
Signed-off-by: Eugen Hristev <[email protected]>
---
arch/arm/configs/multi_v7_defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 8863fa969ede..b768abad8df0 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -639,7 +639,10 @@ CONFIG_VIDEO_S5P_MIPI_CSIS=m
CONFIG_VIDEO_EXYNOS_FIMC_LITE=m
CONFIG_VIDEO_EXYNOS4_FIMC_IS=m
CONFIG_VIDEO_RCAR_VIN=m
+CONFIG_VIDEO_ATMEL_ISC=m
+CONFIG_VIDEO_ATMEL_XISC=m
CONFIG_VIDEO_ATMEL_ISI=m
+CONFIG_VIDEO_MICROCHIP_CSI2DC=m
CONFIG_V4L_MEM2MEM_DRIVERS=y
CONFIG_VIDEO_SAMSUNG_S5P_JPEG=m
CONFIG_VIDEO_SAMSUNG_S5P_MFC=m
--
2.25.1
Remove duplicate comments which are already in place before the struct
definition.
Signed-off-by: Eugen Hristev <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
drivers/media/platform/atmel/atmel-isc.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h
index 07fa6dbf8460..f9ad7ec6bd13 100644
--- a/drivers/media/platform/atmel/atmel-isc.h
+++ b/drivers/media/platform/atmel/atmel-isc.h
@@ -272,7 +272,7 @@ struct isc_device {
struct video_device video_dev;
struct vb2_queue vb2_vidq;
- spinlock_t dma_queue_lock; /* serialize access to dma queue */
+ spinlock_t dma_queue_lock;
struct list_head dma_queue;
struct isc_buffer *cur_frm;
unsigned int sequence;
@@ -289,8 +289,8 @@ struct isc_device {
struct isc_ctrls ctrls;
struct work_struct awb_work;
- struct mutex lock; /* serialize access to file operations */
- spinlock_t awb_lock; /* serialize access to DMA buffers from awb work queue */
+ struct mutex lock;
+ spinlock_t awb_lock;
struct regmap_field *pipeline[ISC_PIPE_LINE_NODE_NUM];
--
2.25.1
Add node for the XISC (eXtended Image Sensor Controller) and CSI2DC
(csi2 demux controller).
These nodes represent the top level of the video capture hardware pipeline
and are directly connected in hardware.
Signed-off-by: Eugen Hristev <[email protected]>
---
arch/arm/boot/dts/sama7g5.dtsi | 49 ++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/arm/boot/dts/sama7g5.dtsi b/arch/arm/boot/dts/sama7g5.dtsi
index eddcfbf4d223..de43f854ce47 100644
--- a/arch/arm/boot/dts/sama7g5.dtsi
+++ b/arch/arm/boot/dts/sama7g5.dtsi
@@ -266,6 +266,55 @@ sdmmc2: mmc@e120c000 {
status = "disabled";
};
+ csi2dc: csi2dc@e1404000 {
+ compatible = "microchip,sama7g5-csi2dc";
+ reg = <0xe1404000 0x500>;
+ clocks = <&pmc PMC_TYPE_PERIPHERAL 34>, <&xisc>;
+ clock-names = "pclk", "scck";
+ assigned-clocks = <&xisc>;
+ assigned-clock-rates = <266000000>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ csi2dc_in: endpoint {
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ csi2dc_out: endpoint {
+ bus-width = <14>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ remote-endpoint = <&xisc_in>;
+ };
+ };
+ };
+ };
+
+ xisc: xisc@e1408000 {
+ compatible = "microchip,sama7g5-isc";
+ reg = <0xe1408000 0x2000>;
+ interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pmc PMC_TYPE_PERIPHERAL 56>;
+ clock-names = "hclock";
+ #clock-cells = <0>;
+ clock-output-names = "isc-mck";
+
+ port {
+ xisc_in: endpoint {
+ bus-type = <5>; /* Parallel */
+ bus-width = <14>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ remote-endpoint = <&csi2dc_out>;
+ };
+ };
+ };
+
pwm: pwm@e1604000 {
compatible = "microchip,sama7g5-pwm", "atmel,sama5d2-pwm";
reg = <0xe1604000 0x4000>;
--
2.25.1
The Microchip XISC supports a bus width of 14 bits.
Add it to the supported bus widths.
Signed-off-by: Eugen Hristev <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/media/microchip,xisc.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/microchip,xisc.yaml b/Documentation/devicetree/bindings/media/microchip,xisc.yaml
index 086e1430af4f..3be8f64c3e21 100644
--- a/Documentation/devicetree/bindings/media/microchip,xisc.yaml
+++ b/Documentation/devicetree/bindings/media/microchip,xisc.yaml
@@ -67,7 +67,7 @@ properties:
remote-endpoint: true
bus-width:
- enum: [8, 9, 10, 11, 12]
+ enum: [8, 9, 10, 11, 12, 14]
default: 12
hsync-active:
--
2.25.1
During experiments with libcamera, it looks like vb2_is_streaming returns
true before our start streaming is called.
Order of operations is streamon -> queue -> start_streaming
ISC would have started the DMA immediately when a buffer is being added
to the vbqueue if the queue is streaming.
It is more safe to start the DMA after the start streaming of the driver is
called.
Thus, even if vb2queue is streaming, add the buffer to the dma queue of the
driver instead of actually starting the DMA process, if the start streaming
has not been called yet.
Tho achieve this, we have to use vb2_start_streaming_called instead of
vb2_is_streaming.
Signed-off-by: Eugen Hristev <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
drivers/media/platform/atmel/atmel-isc-base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
index db15770d5b88..d2cc6c99984f 100644
--- a/drivers/media/platform/atmel/atmel-isc-base.c
+++ b/drivers/media/platform/atmel/atmel-isc-base.c
@@ -442,7 +442,7 @@ static void isc_buffer_queue(struct vb2_buffer *vb)
spin_lock_irqsave(&isc->dma_queue_lock, flags);
if (!isc->cur_frm && list_empty(&isc->dma_queue) &&
- vb2_is_streaming(vb->vb2_queue)) {
+ vb2_start_streaming_called(vb->vb2_queue)) {
isc->cur_frm = buf;
isc_start_dma(isc);
} else
--
2.25.1
Compact the list array to be more readable.
No other changes, only cosmetic.
Signed-off-by: Eugen Hristev <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
.../media/platform/atmel/atmel-sama5d2-isc.c | 51 ++++++----------
.../media/platform/atmel/atmel-sama7g5-isc.c | 60 +++++++------------
2 files changed, 37 insertions(+), 74 deletions(-)
diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
index 025c3e8a7e95..d96ee3373889 100644
--- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c
+++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
@@ -60,56 +60,39 @@
static const struct isc_format sama5d2_controller_formats[] = {
{
.fourcc = V4L2_PIX_FMT_ARGB444,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_ARGB555,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_RGB565,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_ABGR32,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_XBGR32,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_YUV420,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_YUYV,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_YUV422P,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_GREY,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_Y10,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SBGGR8,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SGBRG8,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SGRBG8,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SRGGB8,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SBGGR10,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SGBRG10,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SGRBG10,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SRGGB10,
},
};
diff --git a/drivers/media/platform/atmel/atmel-sama7g5-isc.c b/drivers/media/platform/atmel/atmel-sama7g5-isc.c
index 9dc75eed0098..e07ae188c15f 100644
--- a/drivers/media/platform/atmel/atmel-sama7g5-isc.c
+++ b/drivers/media/platform/atmel/atmel-sama7g5-isc.c
@@ -63,65 +63,45 @@
static const struct isc_format sama7g5_controller_formats[] = {
{
.fourcc = V4L2_PIX_FMT_ARGB444,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_ARGB555,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_RGB565,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_ABGR32,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_XBGR32,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_YUV420,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_UYVY,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_VYUY,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_YUYV,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_YUV422P,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_GREY,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_Y10,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_Y16,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SBGGR8,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SGBRG8,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SGRBG8,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SRGGB8,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SBGGR10,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SGBRG10,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SGRBG10,
- },
- {
+ }, {
.fourcc = V4L2_PIX_FMT_SRGGB10,
},
};
--
2.25.1
The AWB workqueue runs in a kernel thread and needs to be synchronized
w.r.t. the streaming status.
It is possible that streaming is stopped while the AWB workq is running.
In this case it is likely that the check for vb2_start_streaming_called is done
at one point in time, but the AWB computations are done later, including a call
to isc_update_profile, which requires streaming to be started.
Thus , isc_update_profile will fail if during this operation sequence the
streaming was stopped.
To solve this issue, a mutex is added, that will serialize the awb work and
streaming stopping, with the mention that either streaming is stopped
completely including termination of the last frame is done, and after that
the AWB work can check stream status and stop; either first AWB work is
completed and after that the streaming can stop correctly.
The awb spin lock cannot be used since this spinlock is taken in the same
context and using it in the stop streaming will result in a recursion BUG.
Signed-off-by: Eugen Hristev <[email protected]>
Reviewed-by: Jacopo Mondi <[email protected]>
---
drivers/media/platform/atmel/atmel-isc-base.c | 29 ++++++++++++++++---
drivers/media/platform/atmel/atmel-isc.h | 2 ++
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
index 448bf281c61a..ee1dda6707a0 100644
--- a/drivers/media/platform/atmel/atmel-isc-base.c
+++ b/drivers/media/platform/atmel/atmel-isc-base.c
@@ -401,6 +401,7 @@ static void isc_stop_streaming(struct vb2_queue *vq)
struct isc_buffer *buf;
int ret;
+ mutex_lock(&isc->awb_mutex);
v4l2_ctrl_activate(isc->do_wb_ctrl, false);
isc->stop = true;
@@ -410,6 +411,8 @@ static void isc_stop_streaming(struct vb2_queue *vq)
v4l2_err(&isc->v4l2_dev,
"Timeout waiting for end of the capture\n");
+ mutex_unlock(&isc->awb_mutex);
+
/* Disable DMA interrupt */
regmap_write(isc->regmap, ISC_INTDIS, ISC_INT_DDONE);
@@ -1397,10 +1400,6 @@ static void isc_awb_work(struct work_struct *w)
u32 min, max;
int ret;
- /* streaming is not active anymore */
- if (isc->stop)
- return;
-
if (ctrls->hist_stat != HIST_ENABLED)
return;
@@ -1455,7 +1454,24 @@ static void isc_awb_work(struct work_struct *w)
}
regmap_write(regmap, ISC_HIS_CFG + isc->offsets.his,
hist_id | baysel | ISC_HIS_CFG_RAR);
+
+ /*
+ * We have to make sure the streaming has not stopped meanwhile.
+ * ISC requires a frame to clock the internal profile update.
+ * To avoid issues, lock the sequence with a mutex
+ */
+ mutex_lock(&isc->awb_mutex);
+
+ /* streaming is not active anymore */
+ if (isc->stop) {
+ mutex_unlock(&isc->awb_mutex);
+ return;
+ };
+
isc_update_profile(isc);
+
+ mutex_unlock(&isc->awb_mutex);
+
/* if awb has been disabled, we don't need to start another histogram */
if (ctrls->awb)
regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
@@ -1548,6 +1564,7 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl)
*/
v4l2_ctrl_activate(isc->do_wb_ctrl, false);
}
+ mutex_unlock(&isc->awb_mutex);
/* if we have autowhitebalance on, start histogram procedure */
if (ctrls->awb == ISC_WB_AUTO &&
@@ -1740,6 +1757,7 @@ static void isc_async_unbind(struct v4l2_async_notifier *notifier,
{
struct isc_device *isc = container_of(notifier->v4l2_dev,
struct isc_device, v4l2_dev);
+ mutex_destroy(&isc->awb_mutex);
cancel_work_sync(&isc->awb_work);
video_unregister_device(&isc->video_dev);
v4l2_ctrl_handler_free(&isc->ctrls.handler);
@@ -1850,6 +1868,8 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
isc->current_subdev = container_of(notifier,
struct isc_subdev_entity, notifier);
mutex_init(&isc->lock);
+ mutex_init(&isc->awb_mutex);
+
init_completion(&isc->comp);
/* Initialize videobuf2 queue */
@@ -1930,6 +1950,7 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
video_unregister_device(vdev);
isc_async_complete_err:
+ mutex_destroy(&isc->awb_mutex);
mutex_destroy(&isc->lock);
return ret;
}
diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h
index 9cc69c3ae26d..f98f25a55e73 100644
--- a/drivers/media/platform/atmel/atmel-isc.h
+++ b/drivers/media/platform/atmel/atmel-isc.h
@@ -229,6 +229,7 @@ enum isc_scaler_pads {
*
* @lock: lock for serializing userspace file operations
* with ISC operations
+ * @awb_mutex: serialize access to streaming status from awb work queue
* @awb_lock: lock for serializing awb work queue operations
* with DMA/buffer operations
*
@@ -307,6 +308,7 @@ struct isc_device {
struct work_struct awb_work;
struct mutex lock;
+ struct mutex awb_mutex;
spinlock_t awb_lock;
struct regmap_field *pipeline[ISC_PIPE_LINE_NODE_NUM];
--
2.25.1