davinci: VPIF: add DT support

Video capture subdevs may be over I2C and may sleep during xfer, so we cannot do IRQ-disabled locking when calling the subdev.

The IRQ-disabled locking is meant to protect the DMA queue list throughout the rest of the driver, so update the locking in [start|stop]_streaming to protect just this list.

Suggested-by Laurent Pinchart <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

---
 drivers/media/platform/davinci/vpif_capture.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(quoted diff) +0 -0

b/drivers/media/platform/davinci/vpif_capture.c index c24049acd40a..f72a719efb3d 100644

drivers/media/platform/davinci/vpif_capture.c +1 -2

@@ -179,8 +179,6 @@ static int vpif_start_streaming(struct vb2_queue *vq,
 unsigned int count) unsigned long addr, flags;
 	int ret;
 
-	spin_lock_irqsave(&common->irqlock, flags);
-
 	/* Initialize field_id */
 	ch->field_id = 0;
 

@@ -211,6 +209,7 @@ static int vpif_start_streaming(struct vb2_queue *vq,
 unsigned int count) vpif_config_addr(ch, ret);
 
 	/* Get the next frame from the buffer queue */
+	spin_lock_irqsave(&common->irqlock, flags);
 	common->cur_frm = common->next_frm = list_entry(common-
 dma_queue.next,
 				    struct vpif_cap_buffer, list);

drivers/media/platform/davinci/vpif_capture.c +2 -1

 	/* Remove buffer from the buffer queue */

@@ -244,6 +243,7 @@ static int vpif_start_streaming(struct vb2_queue *vq,
 unsigned int count) return 0;
 
 err:
+	spin_lock_irqsave(&common->irqlock, flags);
 	list_for_each_entry_safe(buf, tmp, &common->dma_queue, list) {
 		list_del(&buf->list);
 		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);

@@ -287,7 +287,6 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
 		vpif_dbg(1, debug, "stream off failed in subdev\n");
 
 	/* release all active buffers */
-	spin_lock_irqsave(&common->irqlock, flags);
 	if (common->cur_frm == common->next_frm) {
 		vb2_buffer_done(&common->cur_frm->vb.vb2_buf,
 				VB2_BUF_STATE_ERROR);

@@ -300,6 +299,7 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
 					VB2_BUF_STATE_ERROR);
 	}
 
+	spin_lock_irqsave(&common->irqlock, flags);
 	while (!list_empty(&common->dma_queue)) {
 		common->next_frm = list_entry(common->dma_queue.next,
 						struct vpif_cap_buffer, list);

Hi Kevin,

Thank you for the patch.

Quoted 9 lines · On Tuesday 06 Dec 2016 21:08:24 Kevin Hilman wrote:

Video capture subdevs may be over I2C and may sleep during xfer, so we cannot do IRQ-disabled locking when calling the subdev.

The IRQ-disabled locking is meant to protect the DMA queue list throughout the rest of the driver, so update the locking in [start|stop]_streaming to protect just this list.

Suggested-by Laurent Pinchart <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

I would also add a comment to document the irqlock field as protecting the dma_queue list only. Something like

(quoted diff) +1 -1

-	/* Used in video-buf */
+	/* Protects the dma_queue field */

	spinlock_t irqlock;

With that,

Reviewed-by Laurent Pinchart <[email protected]>

Acked-by Rob Herring <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

---
.../devicetree/bindings/media/ti,da850-vpif.txt | 67 +++++++++++++++++++

1 file changed, 67 insertions(+)

create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file mode 100644
index 000000000000..fa06dfdb6898

Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file +13 -0

@@ -0,0 +1,67 @@
+Texas Instruments VPIF
+----------------------
+
+The TI Video Port InterFace (VPIF) is the primary component for video
+capture and display on the DA850/AM18x family of TI DaVinci/Sitara
+SoCs.
+
+TI Document reference: SPRUH82C, Chapter 35
+http://www.ti.com/lit/pdf/spruh82
+
+Required properties:
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the

device;
+- interrupts: should contain IRQ line for the VPIF

Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file +45 -0

+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+	vpif: vpif@217000 {
+		compatible = "ti,da850-vpif";
+		reg = <0x217000 0x1000>;
+		interrupts = <92>;
+
+		port {
+			vpif_ch0: endpoint@0 {
+				  reg = <0>;
+				  bus-width = <8>;
+				  remote-endpoint = <&composite>;
+			};
+
+			vpif_ch1: endpoint@1 {
+				  reg = <1>;
+				  bus-width = <8>;
+				  data-shift = <8>;
+			};
+		};
+	};
+
+[ ... ]
+
+&i2c0 {
+
+	tvp5147@5d {
+		compatible = "ti,tvp5147";
+		reg = <0x5d>;
+		status = "okay";
+
+		port {
+			composite: endpoint {
+				hsync-active = <1>;
+				vsync-active = <1>;
+				pclk-sample = <0>;
+
+				/* VPIF channel 0 (lower 8-bits) */
+				remote-endpoint = <&vpif_ch0>;
+				bus-width = <8>;
+			};
+		};
+	};
+};

Hi Kevin,

Thank you for the patch.

Quoted 32 lines · On Tuesday 06 Dec 2016 21:08:25 Kevin Hilman wrote:

Acked-by Rob Herring <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

---
.../devicetree/bindings/media/ti,da850-vpif.txt | 67 +++++++++++++++++++

1 file changed, 67 insertions(+)

create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file mode 100644
index 000000000000..fa06dfdb6898

Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file +13 -0

@@ -0,0 +1,67 @@
+Texas Instruments VPIF
+----------------------
+
+The TI Video Port InterFace (VPIF) is the primary component for video
+capture and display on the DA850/AM18x family of TI DaVinci/Sitara
+SoCs.
+
+TI Document reference: SPRUH82C, Chapter 35
+http://www.ti.com/lit/pdf/spruh82
+
+Required properties:
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the

device;
+- interrupts: should contain IRQ line for the VPIF

Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file +2 -0

+
+Video Capture:

Documentation/devicetree/bindings/media/ti,da850-vpif.txt line 21

Quoted 5 lines

Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file +5 -0

+
+VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
+single 16-bit channel.  It should contain at least one port child node
+with child 'endpoint' node. Please refer to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.

You might want to clarify how endpoints are use in the two cases. Apart from that,

Quoted 37 lines

Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file +37 -0

+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+	vpif: vpif at 217000 {
+		compatible = "ti,da850-vpif";
+		reg = <0x217000 0x1000>;
+		interrupts = <92>;
+
+		port {
+			vpif_ch0: endpoint at 0 {
+				  reg = <0>;
+				  bus-width = <8>;
+				  remote-endpoint = <&composite>;
+			};
+
+			vpif_ch1: endpoint at 1 {
+				  reg = <1>;
+				  bus-width = <8>;
+				  data-shift = <8>;
+			};
+		};
+	};
+
+[ ... ]
+
+&i2c0 {
+
+	tvp5147 at 5d {
+		compatible = "ti,tvp5147";
+		reg = <0x5d>;
+		status = "okay";
+
+		port {
+			composite: endpoint {
+				hsync-active = <1>;
+				vsync-active = <1>;
+				pclk-sample = <0>;

Documentation/devicetree/bindings/media/ti,da850-vpif.txt line 67

Quoted 8 lines

Documentation/devicetree/bindings/media/ti,da850-vpif.txt new file +8 -0

+
+				/* VPIF channel 0 (lower 8-bits) */
+				remote-endpoint = <&vpif_ch0>;
+				bus-width = <8>;
+			};
+		};
+	};
+};