[PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support

[PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support

[Jagan Teki]

This series support CSI on Allwinner A64.

Tested 640x480, 320x240, 720p, 1080p resolutions UYVY8_2X8 format.

Changes for v4:
- update the compatible string order
- add proper commit message
- included BPI-M64 patch
- skipped amarula-a64 patch
Changes for v3:
- update dt-bindings for A64
- set mod clock via csi driver
- remove assign clocks from dtsi
- remove i2c-gpio opendrian
- fix avdd and dovdd supplies
- remove vcc-csi pin group supply

Note: This series created on top of H3 changes [1]

[1] https://patchwork.kernel.org/cover/10705905/

Any inputs,
Jagan. 

Jagan Teki (6):
  dt-bindings: media: sun6i: Add A64 CSI compatible
  media: sun6i: Add A64 compatible support
  media: sun6i: Update default CSI_SCLK for A64
  arm64: dts: allwinner: a64: Add A64 CSI controller
  arm64: dts: allwinner: a64: Add pinmux setting for CSI MCLK on PE1
  [DO NOT MERGE] arm64: dts: allwinner: bananapi-m64: Add HDF5640 camera module

 .../devicetree/bindings/media/sun6i-csi.txt   |  1 +
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 65 +++++++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 25 +++++++
 .../platform/sunxi/sun6i-csi/sun6i_csi.c      |  6 ++
 4 files changed, 97 insertions(+)

-- 
2.18.0.321.gffc6fa0e3

[PATCH v4 1/6] dt-bindings: media: sun6i: Add A64 CSI compatible

[Jagan Teki]

Allwinner A64 CSI is a single channel time-multiplexed BT.656
protocol interface.

Add separate compatible string for A64 since it require explicit
change in sun6i_csi driver to update default CSI_SCLK rate.

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
---
 Documentation/devicetree/bindings/media/sun6i-csi.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt
index cc37cf7fd051..0dd540bb03db 100644
--- a/Documentation/devicetree/bindings/media/sun6i-csi.txt
+++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
@@ -8,6 +8,7 @@ Required properties:
     * "allwinner,sun6i-a31-csi"
     * "allwinner,sun8i-h3-csi"
     * "allwinner,sun8i-v3s-csi"
+    * "allwinner,sun50i-a64-csi"
   - reg: base address and size of the memory-mapped region.
   - interrupts: interrupt associated to this IP
   - clocks: phandles to the clocks feeding the CSI
-- 
2.18.0.321.gffc6fa0e3

[PATCH v4 2/6] media: sun6i: Add A64 compatible support

[Jagan Teki]

Add compatible string for Allwinner A64 CSI.

A64 CSI has similar features as like in H3, but the CSI_SCLK
need to update it to 300MHz than default clock rate.

A64 BSP is also operating same rate as default csi clock.

Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
---
 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
index ee882b66a5ea..9ff61896e4bb 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -895,6 +895,7 @@ static const struct of_device_id sun6i_csi_of_match[] = {
 	{ .compatible = "allwinner,sun6i-a31-csi", },
 	{ .compatible = "allwinner,sun8i-h3-csi", },
 	{ .compatible = "allwinner,sun8i-v3s-csi", },
+	{ .compatible = "allwinner,sun50i-a64-csi", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, sun6i_csi_of_match);
-- 
2.18.0.321.gffc6fa0e3

[PATCH v4 3/6] media: sun6i: Update default CSI_SCLK for A64

[Jagan Teki]

Unfortunately A64 CSI cannot work with default CSI_SCLK rate.

A64 BSP is using 300MHz clock rate as default csi clock,
so sun6i_csi require explicit change to update CSI_SCLK
rate to 300MHZ for A64 SoC's.

So, set the clk_mod to 300MHz only for A64.

Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
---
 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
index 9ff61896e4bb..91470edf7581 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -822,6 +822,11 @@ static int sun6i_csi_resource_request(struct sun6i_csi_dev *sdev,
 		return PTR_ERR(sdev->clk_mod);
 	}
 
+	/* A64 require 300MHz mod clock to operate properly */
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "allwinner,sun50i-a64-csi"))
+		clk_set_rate_exclusive(sdev->clk_mod, 300000000);
+
 	sdev->clk_ram = devm_clk_get(&pdev->dev, "ram");
 	if (IS_ERR(sdev->clk_ram)) {
 		dev_err(&pdev->dev, "Unable to acquire dram-csi clock\n");
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v4 3/6] media: sun6i: Update default CSI_SCLK for A64

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

On Tue, Dec 18, 2018 at 05:03:17PM +0530, Jagan Teki wrote:
> Unfortunately A64 CSI cannot work with default CSI_SCLK rate.
> 
> A64 BSP is using 300MHz clock rate as default csi clock,
> so sun6i_csi require explicit change to update CSI_SCLK
> rate to 300MHZ for A64 SoC's.
> 
> So, set the clk_mod to 300MHz only for A64.
> 
> Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> ---
>  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> index 9ff61896e4bb..91470edf7581 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> @@ -822,6 +822,11 @@ static int sun6i_csi_resource_request(struct sun6i_csi_dev *sdev,
>  		return PTR_ERR(sdev->clk_mod);
>  	}
>  
> +	/* A64 require 300MHz mod clock to operate properly */
> +	if (of_device_is_compatible(pdev->dev.of_node,
> +				    "allwinner,sun50i-a64-csi"))
> +		clk_set_rate_exclusive(sdev->clk_mod, 300000000);
> +

If you're using clk_set_rate_exclusive, you need to put back the
"exclusive" reference once you're not using the clock.

Doing it here is not really optimal either, since you'll put a
constraint on the system (maintaining that clock at 300MHz), while
it's not in use.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v4 3/6] media: sun6i: Update default CSI_SCLK for A64

[Jagan Teki]

On Tue, Dec 18, 2018 at 8:53 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Tue, Dec 18, 2018 at 05:03:17PM +0530, Jagan Teki wrote:
> > Unfortunately A64 CSI cannot work with default CSI_SCLK rate.
> >
> > A64 BSP is using 300MHz clock rate as default csi clock,
> > so sun6i_csi require explicit change to update CSI_SCLK
> > rate to 300MHZ for A64 SoC's.
> >
> > So, set the clk_mod to 300MHz only for A64.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > index 9ff61896e4bb..91470edf7581 100644
> > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > @@ -822,6 +822,11 @@ static int sun6i_csi_resource_request(struct sun6i_csi_dev *sdev,
> >               return PTR_ERR(sdev->clk_mod);
> >       }
> >
> > +     /* A64 require 300MHz mod clock to operate properly */
> > +     if (of_device_is_compatible(pdev->dev.of_node,
> > +                                 "allwinner,sun50i-a64-csi"))
> > +             clk_set_rate_exclusive(sdev->clk_mod, 300000000);
> > +
>
> If you're using clk_set_rate_exclusive, you need to put back the
> "exclusive" reference once you're not using the clock.
>
> Doing it here is not really optimal either, since you'll put a
> constraint on the system (maintaining that clock at 300MHz), while
> it's not in use.

I think we can handle via clk_rate_exclusive_put for those errors
cases? If I'm not wrong

Re: [PATCH v4 3/6] media: sun6i: Update default CSI_SCLK for A64

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]

On Tue, Dec 18, 2018 at 09:08:17PM +0530, Jagan Teki wrote:
> On Tue, Dec 18, 2018 at 8:53 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> >
> > On Tue, Dec 18, 2018 at 05:03:17PM +0530, Jagan Teki wrote:
> > > Unfortunately A64 CSI cannot work with default CSI_SCLK rate.
> > >
> > > A64 BSP is using 300MHz clock rate as default csi clock,
> > > so sun6i_csi require explicit change to update CSI_SCLK
> > > rate to 300MHZ for A64 SoC's.
> > >
> > > So, set the clk_mod to 300MHz only for A64.
> > >
> > > Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> > > ---
> > >  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > index 9ff61896e4bb..91470edf7581 100644
> > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > @@ -822,6 +822,11 @@ static int sun6i_csi_resource_request(struct sun6i_csi_dev *sdev,
> > >               return PTR_ERR(sdev->clk_mod);
> > >       }
> > >
> > > +     /* A64 require 300MHz mod clock to operate properly */
> > > +     if (of_device_is_compatible(pdev->dev.of_node,
> > > +                                 "allwinner,sun50i-a64-csi"))
> > > +             clk_set_rate_exclusive(sdev->clk_mod, 300000000);
> > > +
> >
> > If you're using clk_set_rate_exclusive, you need to put back the
> > "exclusive" reference once you're not using the clock.
> >
> > Doing it here is not really optimal either, since you'll put a
> > constraint on the system (maintaining that clock at 300MHz), while
> > it's not in use.
> 
> I think we can handle via clk_rate_exclusive_put for those errors
> cases? If I'm not wrong

Yes, but it's not only for the error case, it's also for the inactive
case.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v4 3/6] media: sun6i: Update default CSI_SCLK for A64

[Jagan Teki]

On Wed, Dec 19, 2018 at 3:37 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
>
> On Tue, Dec 18, 2018 at 09:08:17PM +0530, Jagan Teki wrote:
> > On Tue, Dec 18, 2018 at 8:53 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > >
> > > On Tue, Dec 18, 2018 at 05:03:17PM +0530, Jagan Teki wrote:
> > > > Unfortunately A64 CSI cannot work with default CSI_SCLK rate.
> > > >
> > > > A64 BSP is using 300MHz clock rate as default csi clock,
> > > > so sun6i_csi require explicit change to update CSI_SCLK
> > > > rate to 300MHZ for A64 SoC's.
> > > >
> > > > So, set the clk_mod to 300MHz only for A64.
> > > >
> > > > Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
> > > > ---
> > > >  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > index 9ff61896e4bb..91470edf7581 100644
> > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > @@ -822,6 +822,11 @@ static int sun6i_csi_resource_request(struct sun6i_csi_dev *sdev,
> > > >               return PTR_ERR(sdev->clk_mod);
> > > >       }
> > > >
> > > > +     /* A64 require 300MHz mod clock to operate properly */
> > > > +     if (of_device_is_compatible(pdev->dev.of_node,
> > > > +                                 "allwinner,sun50i-a64-csi"))
> > > > +             clk_set_rate_exclusive(sdev->clk_mod, 300000000);
> > > > +
> > >
> > > If you're using clk_set_rate_exclusive, you need to put back the
> > > "exclusive" reference once you're not using the clock.
> > >
> > > Doing it here is not really optimal either, since you'll put a
> > > constraint on the system (maintaining that clock at 300MHz), while
> > > it's not in use.
> >
> > I think we can handle via clk_rate_exclusive_put for those errors
> > cases? If I'm not wrong
>
> Yes, but it's not only for the error case, it's also for the inactive
> case.

Yes, I will try to add this next version.

[PATCH v4 4/6] arm64: dts: allwinner: a64: Add A64 CSI controller

[Jagan Teki]

Add dts node details for Allwinner A64 CSI controller.

A64 CSI has similar features as like in H3, but the CSI_SCLK
need to update it to 300MHz than default clock rate.

Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 384c417cb7a2..89a0deb3fe6a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -532,6 +532,12 @@
 			interrupt-controller;
 			#interrupt-cells = <3>;
 
+			csi_pins: csi-pins {
+				pins = "PE0", "PE2", "PE3", "PE4", "PE5", "PE6",
+				       "PE7", "PE8", "PE9", "PE10", "PE11";
+				function = "csi0";
+			};
+
 			i2c0_pins: i2c0_pins {
 				pins = "PH0", "PH1";
 				function = "i2c0";
@@ -899,6 +905,20 @@
 			status = "disabled";
 		};
 
+		csi: csi@1cb0000 {
+			compatible = "allwinner,sun50i-a64-csi";
+			reg = <0x01cb0000 0x1000>;
+			interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_CSI>,
+				 <&ccu CLK_CSI_SCLK>,
+				 <&ccu CLK_DRAM_CSI>;
+			clock-names = "bus", "mod", "ram";
+			resets = <&ccu RST_BUS_CSI>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&csi_pins>;
+			status = "disabled";
+		};
+
 		hdmi: hdmi@1ee0000 {
 			compatible = "allwinner,sun50i-a64-dw-hdmi",
 				     "allwinner,sun8i-a83t-dw-hdmi";
-- 
2.18.0.321.gffc6fa0e3

[PATCH v4 5/6] arm64: dts: allwinner: a64: Add pinmux setting for CSI MCLK on PE1

[Jagan Teki]

Some camera modules have the SoC feeding a master clock to the sensor
instead of having a standalone crystal. This clock signal is generated
from the clock control unit and output from the CSI MCLK function of
pin PE1.

Add a pinmux setting for it for camera sensors to reference.

Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 89a0deb3fe6a..dd5740bc3fc9 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -538,6 +538,11 @@
 				function = "csi0";
 			};
 
+			csi_mclk_pin: csi-mclk {
+				pins = "PE1";
+				function = "csi0";
+			};
+
 			i2c0_pins: i2c0_pins {
 				pins = "PH0", "PH1";
 				function = "i2c0";
-- 
2.18.0.321.gffc6fa0e3

[DO NOT MERGE] [PATCH v4 6/6] arm64: dts: allwinner: bananapi-m64: Add HDF5640 camera module

[Jagan Teki]

Bananapi M64 comes with an optional sensor based on the ov5640,
add support for it with below pin information.

- PE13, PE12 via i2c-gpio bitbanging
- CLK_CSI_MCLK as external clock
- PE1 as external clock pin muxing
- DLDO3 as AVDD supply
- ALDO1 as DOVDD supply
- ELDO3 as DVDD supply
- PE16 gpio for reset pin
- PE17 gpio for powerdown pin

Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
---
 .../dts/allwinner/sun50i-a64-bananapi-m64.dts | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index 83e30e0afe5b..c185ceec8c81 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -60,6 +60,41 @@
 		stdout-path = "serial0:115200n8";
 	};
 
+	i2c-csi {
+		compatible = "i2c-gpio";
+		sda-gpios = <&pio 4 13 GPIO_ACTIVE_HIGH>; /* CSI0-SDA: PE13 */
+		scl-gpios = <&pio 4 12 GPIO_ACTIVE_HIGH>; /* CSI0-SCK: PE12 */
+		i2c-gpio,delay-us = <5>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ov5640: camera@3c {
+			compatible = "ovti,ov5640";
+			reg = <0x3c>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&csi_mclk_pin>;
+			clocks = <&ccu CLK_CSI_MCLK>;
+			clock-names = "xclk";
+
+			AVDD-supply = <&reg_dldo3>;
+			DOVDD-supply = <&reg_aldo1>;
+			DVDD-supply = <&reg_eldo3>;
+			reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* CSI0-RST: PE16 */
+			powerdown-gpios = <&pio 4 17 GPIO_ACTIVE_HIGH>; /* CSI0-PWDN: PE17 */
+
+			port {
+				ov5640_ep: endpoint {
+					remote-endpoint = <&csi_ep>;
+					bus-width = <8>;
+					hsync-active = <1>; /* Active high */
+					vsync-active = <0>; /* Active low */
+					data-active = <1>;  /* Active high */
+					pclk-sample = <1>;  /* Rising */
+				};
+			};
+		};
+	};
+
 	hdmi-connector {
 		compatible = "hdmi-connector";
 		type = "a";
@@ -106,6 +141,24 @@
 	status = "okay";
 };
 
+&csi {
+	status = "okay";
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		csi_ep: endpoint {
+			remote-endpoint = <&ov5640_ep>;
+			bus-width = <8>;
+			hsync-active = <1>; /* Active high */
+			vsync-active = <0>; /* Active low */
+			data-active = <1>;  /* Active high */
+			pclk-sample = <1>;  /* Rising */
+		};
+	};
+};
+
 &dai {
 	status = "okay";
 };
@@ -296,6 +349,12 @@
 	regulator-name = "vcc-wifi";
 };
 
+&reg_dldo3 {
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "avdd-csi";
+};
+
 &reg_dldo4 {
 	regulator-min-microvolt = <1800000>;
 	regulator-max-microvolt = <3300000>;
@@ -313,6 +372,12 @@
 	regulator-name = "cpvdd";
 };
 
+&reg_eldo3 {
+	regulator-min-microvolt = <1500000>;
+	regulator-max-microvolt = <1500000>;
+	regulator-name = "dvdd-csi";
+};
+
 &reg_fldo1 {
 	regulator-min-microvolt = <1200000>;
 	regulator-max-microvolt = <1200000>;
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On Tue, Dec 18, 2018 at 05:03:14PM +0530, Jagan Teki wrote:
> This series support CSI on Allwinner A64.
> 
> Tested 640x480, 320x240, 720p, 1080p resolutions UYVY8_2X8 format.
> 
> Changes for v4:
> - update the compatible string order
> - add proper commit message
> - included BPI-M64 patch
> - skipped amarula-a64 patch
> Changes for v3:
> - update dt-bindings for A64
> - set mod clock via csi driver
> - remove assign clocks from dtsi
> - remove i2c-gpio opendrian
> - fix avdd and dovdd supplies
> - remove vcc-csi pin group supply
> 
> Note: This series created on top of H3 changes [1]
> 
> [1] https://patchwork.kernel.org/cover/10705905/

You had memory corruption before, how was this fixed?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support

[Jagan Teki]

On Tue, Dec 18, 2018 at 8:51 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Tue, Dec 18, 2018 at 05:03:14PM +0530, Jagan Teki wrote:
> > This series support CSI on Allwinner A64.
> >
> > Tested 640x480, 320x240, 720p, 1080p resolutions UYVY8_2X8 format.
> >
> > Changes for v4:
> > - update the compatible string order
> > - add proper commit message
> > - included BPI-M64 patch
> > - skipped amarula-a64 patch
> > Changes for v3:
> > - update dt-bindings for A64
> > - set mod clock via csi driver
> > - remove assign clocks from dtsi
> > - remove i2c-gpio opendrian
> > - fix avdd and dovdd supplies
> > - remove vcc-csi pin group supply
> >
> > Note: This series created on top of H3 changes [1]
> >
> > [1] https://patchwork.kernel.org/cover/10705905/
>
> You had memory corruption before, how was this fixed?

Memory corruption observed with default 600MHz on 1080p. It worked
fine on BPI-M64 (with 300MHz)

Re: [PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

On Tue, Dec 18, 2018 at 08:58:22PM +0530, Jagan Teki wrote:
> On Tue, Dec 18, 2018 at 8:51 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Tue, Dec 18, 2018 at 05:03:14PM +0530, Jagan Teki wrote:
> > > This series support CSI on Allwinner A64.
> > >
> > > Tested 640x480, 320x240, 720p, 1080p resolutions UYVY8_2X8 format.
> > >
> > > Changes for v4:
> > > - update the compatible string order
> > > - add proper commit message
> > > - included BPI-M64 patch
> > > - skipped amarula-a64 patch
> > > Changes for v3:
> > > - update dt-bindings for A64
> > > - set mod clock via csi driver
> > > - remove assign clocks from dtsi
> > > - remove i2c-gpio opendrian
> > > - fix avdd and dovdd supplies
> > > - remove vcc-csi pin group supply
> > >
> > > Note: This series created on top of H3 changes [1]
> > >
> > > [1] https://patchwork.kernel.org/cover/10705905/
> >
> > You had memory corruption before, how was this fixed?
> 
> Memory corruption observed with default 600MHz on 1080p. It worked
> fine on BPI-M64 (with 300MHz)

I don't get it. In the previous version of those patches, you were
mentionning you were still having this issue, even though you had the
clock running at 300MHz, and then you tried to convince us to merge
the patches nonetheless.

Why would you say that then if that issue was fixed?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support

[Michael Nazzareno Trimarchi]

Hi Maxime

On Wed, Dec 19, 2018 at 11:25 AM Maxime Ripard
<maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
>
> On Tue, Dec 18, 2018 at 08:58:22PM +0530, Jagan Teki wrote:
> > On Tue, Dec 18, 2018 at 8:51 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > >
> > > On Tue, Dec 18, 2018 at 05:03:14PM +0530, Jagan Teki wrote:
> > > > This series support CSI on Allwinner A64.
> > > >
> > > > Tested 640x480, 320x240, 720p, 1080p resolutions UYVY8_2X8 format.
> > > >
> > > > Changes for v4:
> > > > - update the compatible string order
> > > > - add proper commit message
> > > > - included BPI-M64 patch
> > > > - skipped amarula-a64 patch
> > > > Changes for v3:
> > > > - update dt-bindings for A64
> > > > - set mod clock via csi driver
> > > > - remove assign clocks from dtsi
> > > > - remove i2c-gpio opendrian
> > > > - fix avdd and dovdd supplies
> > > > - remove vcc-csi pin group supply
> > > >
> > > > Note: This series created on top of H3 changes [1]
> > > >
> > > > [1] https://patchwork.kernel.org/cover/10705905/
> > >
> > > You had memory corruption before, how was this fixed?
> >
> > Memory corruption observed with default 600MHz on 1080p. It worked
> > fine on BPI-M64 (with 300MHz)
>
> I don't get it. In the previous version of those patches, you were
> mentionning you were still having this issue, even though you had the
> clock running at 300MHz, and then you tried to convince us to merge
> the patches nonetheless.
>
> Why would you say that then if that issue was fixed?

It's simple, we have a full working platform with ov5640 that support
all the resolutions, that can be test by anyone and
we have an industrial product that has some problem on high xvclk
because it can not give us a clear image but this is limited on
another design and another camera module vendor. Problem is not in the
kernel code but it's just on relic design. We are getting
the information on that module and see if inside is really using the
xvclk or it's clocked by some other things. We have only the schematic
up to the connector.

Michael

>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

Re: [PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support

[Jagan Teki]

On Wed, Dec 19, 2018 at 3:55 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
>
> On Tue, Dec 18, 2018 at 08:58:22PM +0530, Jagan Teki wrote:
> > On Tue, Dec 18, 2018 at 8:51 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > >
> > > On Tue, Dec 18, 2018 at 05:03:14PM +0530, Jagan Teki wrote:
> > > > This series support CSI on Allwinner A64.
> > > >
> > > > Tested 640x480, 320x240, 720p, 1080p resolutions UYVY8_2X8 format.
> > > >
> > > > Changes for v4:
> > > > - update the compatible string order
> > > > - add proper commit message
> > > > - included BPI-M64 patch
> > > > - skipped amarula-a64 patch
> > > > Changes for v3:
> > > > - update dt-bindings for A64
> > > > - set mod clock via csi driver
> > > > - remove assign clocks from dtsi
> > > > - remove i2c-gpio opendrian
> > > > - fix avdd and dovdd supplies
> > > > - remove vcc-csi pin group supply
> > > >
> > > > Note: This series created on top of H3 changes [1]
> > > >
> > > > [1] https://patchwork.kernel.org/cover/10705905/
> > >
> > > You had memory corruption before, how was this fixed?
> >
> > Memory corruption observed with default 600MHz on 1080p. It worked
> > fine on BPI-M64 (with 300MHz)
>
> I don't get it. In the previous version of those patches, you were
> mentionning you were still having this issue, even though you had the
> clock running at 300MHz, and then you tried to convince us to merge
> the patches nonetheless.
>
> Why would you say that then if that issue was fixed?

Previous version has A64-Relic board, which has some xclk issue on
sensor side wrt 1080p. I have tried 300MHz on the same hardware, it's
failing to capture on 30fps and so I tried 600MHz(which is default) on
the same configuration but it encounter memory corruption.

So, for checking whether there is an issue with hardware on A64-Relic
I moved with BPI-M64 dev board. which is working 1080p with 300MHz, ie
reason I have not included A64-Relic on this version and included
BPI-M64. We processed A64-Relic to hardware team to figure out the
clock and once ie fixed I'm planning to send DTS patch for that.

This is overall summary, hope you understand.

Re: [PATCH v4 0/6] media/sun6i: Allwinner A64 CSI support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

65;5402;1c
On Wed, Dec 19, 2018 at 04:11:50PM +0530, Jagan Teki wrote:
> On Wed, Dec 19, 2018 at 3:55 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> >
> > On Tue, Dec 18, 2018 at 08:58:22PM +0530, Jagan Teki wrote:
> > > On Tue, Dec 18, 2018 at 8:51 PM Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > >
> > > > On Tue, Dec 18, 2018 at 05:03:14PM +0530, Jagan Teki wrote:
> > > > > This series support CSI on Allwinner A64.
> > > > >
> > > > > Tested 640x480, 320x240, 720p, 1080p resolutions UYVY8_2X8 format.
> > > > >
> > > > > Changes for v4:
> > > > > - update the compatible string order
> > > > > - add proper commit message
> > > > > - included BPI-M64 patch
> > > > > - skipped amarula-a64 patch
> > > > > Changes for v3:
> > > > > - update dt-bindings for A64
> > > > > - set mod clock via csi driver
> > > > > - remove assign clocks from dtsi
> > > > > - remove i2c-gpio opendrian
> > > > > - fix avdd and dovdd supplies
> > > > > - remove vcc-csi pin group supply
> > > > >
> > > > > Note: This series created on top of H3 changes [1]
> > > > >
> > > > > [1] https://patchwork.kernel.org/cover/10705905/
> > > >
> > > > You had memory corruption before, how was this fixed?
> > >
> > > Memory corruption observed with default 600MHz on 1080p. It worked
> > > fine on BPI-M64 (with 300MHz)
> >
> > I don't get it. In the previous version of those patches, you were
> > mentionning you were still having this issue, even though you had the
> > clock running at 300MHz, and then you tried to convince us to merge
> > the patches nonetheless.
> >
> > Why would you say that then if that issue was fixed?
> 
> Previous version has A64-Relic board, which has some xclk issue on
> sensor side wrt 1080p. I have tried 300MHz on the same hardware, it's
> failing to capture on 30fps and so I tried 600MHz(which is default) on
> the same configuration but it encounter memory corruption.
> 
> So, for checking whether there is an issue with hardware on A64-Relic
> I moved with BPI-M64 dev board. which is working 1080p with 300MHz, ie
> reason I have not included A64-Relic on this version and included
> BPI-M64. We processed A64-Relic to hardware team to figure out the
> clock and once ie fixed I'm planning to send DTS patch for that.
> 
> This is overall summary, hope you understand.

Ok, great, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com