* [RFC PATCH v9 1/9] arm64: tegra: Fix sor powergate clocks and reset
2020-04-22 6:18 [RFC PATCH v9 0/9] Add Tegra driver for video capture Sowjanya Komatineni
@ 2020-04-22 6:18 ` Sowjanya Komatineni
2020-04-22 6:18 ` [RFC PATCH v9 2/9] arm64: tegra: Add reset-cells to mc Sowjanya Komatineni
` (7 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 6:18 UTC (permalink / raw)
To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
Tegra210 device tree lists csi clock and reset under SOR powergate
node.
But Tegra210 has csicil in SOR partition and csi in VENC partition.
So, this patch includes fix for sor powergate node.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 64c46ce..d0eff92 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -796,7 +796,9 @@
pd_sor: sor {
clocks = <&tegra_car TEGRA210_CLK_SOR0>,
<&tegra_car TEGRA210_CLK_SOR1>,
- <&tegra_car TEGRA210_CLK_CSI>,
+ <&tegra_car TEGRA210_CLK_CILAB>,
+ <&tegra_car TEGRA210_CLK_CILCD>,
+ <&tegra_car TEGRA210_CLK_CILE>,
<&tegra_car TEGRA210_CLK_DSIA>,
<&tegra_car TEGRA210_CLK_DSIB>,
<&tegra_car TEGRA210_CLK_DPAUX>,
@@ -804,7 +806,6 @@
<&tegra_car TEGRA210_CLK_MIPI_CAL>;
resets = <&tegra_car TEGRA210_CLK_SOR0>,
<&tegra_car TEGRA210_CLK_SOR1>,
- <&tegra_car TEGRA210_CLK_CSI>,
<&tegra_car TEGRA210_CLK_DSIA>,
<&tegra_car TEGRA210_CLK_DSIB>,
<&tegra_car TEGRA210_CLK_DPAUX>,
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [RFC PATCH v9 2/9] arm64: tegra: Add reset-cells to mc
2020-04-22 6:18 [RFC PATCH v9 0/9] Add Tegra driver for video capture Sowjanya Komatineni
2020-04-22 6:18 ` [RFC PATCH v9 1/9] arm64: tegra: Fix sor powergate clocks and reset Sowjanya Komatineni
@ 2020-04-22 6:18 ` Sowjanya Komatineni
2020-04-22 6:18 ` [RFC PATCH v9 3/9] dt-bindings: clock: tegra: Add clk id for CSI TPG clock Sowjanya Komatineni
` (6 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 6:18 UTC (permalink / raw)
To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
Tegra210 device tree is missing reset-cells property for mc node.
This patch fixes it.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index d0eff92..5b1dfd8 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -894,6 +894,7 @@
interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
#iommu-cells = <1>;
+ #reset-cells = <1>;
};
sata@70020000 {
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [RFC PATCH v9 3/9] dt-bindings: clock: tegra: Add clk id for CSI TPG clock
2020-04-22 6:18 [RFC PATCH v9 0/9] Add Tegra driver for video capture Sowjanya Komatineni
2020-04-22 6:18 ` [RFC PATCH v9 1/9] arm64: tegra: Fix sor powergate clocks and reset Sowjanya Komatineni
2020-04-22 6:18 ` [RFC PATCH v9 2/9] arm64: tegra: Add reset-cells to mc Sowjanya Komatineni
@ 2020-04-22 6:18 ` Sowjanya Komatineni
2020-04-22 6:18 ` [RFC PATCH v9 4/9] clk: tegra: Add Tegra210 CSI TPG clock gate Sowjanya Komatineni
` (5 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 6:18 UTC (permalink / raw)
To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
Tegra210 uses PLLD out internally for CSI TPG.
This patch adds clk id for this CSI TPG clock from PLLD.
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
include/dt-bindings/clock/tegra210-car.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/dt-bindings/clock/tegra210-car.h b/include/dt-bindings/clock/tegra210-car.h
index 7a8f10b..d8909e0 100644
--- a/include/dt-bindings/clock/tegra210-car.h
+++ b/include/dt-bindings/clock/tegra210-car.h
@@ -351,7 +351,7 @@
#define TEGRA210_CLK_PLL_P_OUT_XUSB 317
#define TEGRA210_CLK_XUSB_SSP_SRC 318
#define TEGRA210_CLK_PLL_RE_OUT1 319
-/* 320 */
+#define TEGRA210_CLK_CSI_TPG 320
/* 321 */
#define TEGRA210_CLK_ISP 322
#define TEGRA210_CLK_PLL_A_OUT_ADSP 323
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [RFC PATCH v9 4/9] clk: tegra: Add Tegra210 CSI TPG clock gate
2020-04-22 6:18 [RFC PATCH v9 0/9] Add Tegra driver for video capture Sowjanya Komatineni
` (2 preceding siblings ...)
2020-04-22 6:18 ` [RFC PATCH v9 3/9] dt-bindings: clock: tegra: Add clk id for CSI TPG clock Sowjanya Komatineni
@ 2020-04-22 6:18 ` Sowjanya Komatineni
2020-04-22 6:18 ` [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings Sowjanya Komatineni
` (4 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 6:18 UTC (permalink / raw)
To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
Tegra210 CSI hardware internally uses PLLD for internal test pattern
generator logic.
PLLD_BASE register in CAR has a bit CSI_CLK_SOURCE to enable PLLD
out to CSI during TPG mode.
This patch adds this CSI TPG clock gate to Tegra210 clock driver
to allow Tegra video driver to ungate CSI TPG clock during TPG mode
and gate during non TPG mode.
Acked-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-tegra210.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index defe3b7..81a879b 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -3035,6 +3035,13 @@ static __init void tegra210_periph_clk_init(void __iomem *clk_base,
periph_clk_enb_refcnt);
clks[TEGRA210_CLK_DSIB] = clk;
+ /* csi_tpg */
+ clk = clk_register_gate(NULL, "csi_tpg", "pll_d",
+ CLK_SET_RATE_PARENT, clk_base + PLLD_BASE,
+ 23, 0, &pll_d_lock);
+ clk_register_clkdev(clk, "csi_tpg", NULL);
+ clks[TEGRA210_CLK_CSI_TPG] = clk;
+
/* la */
clk = tegra_clk_register_periph("la", la_parents,
ARRAY_SIZE(la_parents), &tegra210_la, clk_base,
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings
2020-04-22 6:18 [RFC PATCH v9 0/9] Add Tegra driver for video capture Sowjanya Komatineni
` (3 preceding siblings ...)
2020-04-22 6:18 ` [RFC PATCH v9 4/9] clk: tegra: Add Tegra210 CSI TPG clock gate Sowjanya Komatineni
@ 2020-04-22 6:18 ` Sowjanya Komatineni
2020-04-22 17:20 ` Laurent Pinchart
2020-04-22 6:18 ` [RFC PATCH v9 7/9] MAINTAINERS: Add Tegra Video driver section Sowjanya Komatineni
` (3 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 6:18 UTC (permalink / raw)
To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
Tegra contains VI controller which can support up to 6 MIPI CSI
camera sensors.
Each Tegra CSI port from CSI unit can be one-to-one mapper to
VI channel and can capture from an external camera sensor or
from built-in test pattern generator.
This patch adds dt-bindings for Tegra VI and CSI.
Acked-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
.../display/tegra/nvidia,tegra20-host1x.txt | 73 ++++++++++++++++++----
1 file changed, 60 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
index 9999255..4731921 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
@@ -40,14 +40,30 @@ of the following host1x client modules:
Required properties:
- compatible: "nvidia,tegra<chip>-vi"
- - reg: Physical base address and length of the controller's registers.
+ - reg: Physical base address and length of the controller registers.
- interrupts: The interrupt outputs from the controller.
- - clocks: Must contain one entry, for the module clock.
+ - clocks: clocks: Must contain one entry, for the module clock.
See ../clocks/clock-bindings.txt for details.
- - resets: Must contain an entry for each entry in reset-names.
- See ../reset/reset.txt for details.
- - reset-names: Must include the following entries:
- - vi
+ - Tegra20/Tegra30/Tegra114/Tegra124:
+ - resets: Must contain an entry for each entry in reset-names.
+ See ../reset/reset.txt for details.
+ - reset-names: Must include the following entries:
+ - vi
+ - Tegra210:
+ - power-domains: Must include venc powergate node as vi is in VE partition.
+ - Tegra210 has CSI part of VI sharing same host interface and register space.
+ So, VI device node should have CSI child node.
+
+ - csi: mipi csi interface to vi
+
+ Required properties:
+ - compatible: "nvidia,tegra210-csi"
+ - reg: Physical base address offset to parent and length of the controller
+ registers.
+ - clocks: Must contain entries csi, cilab, cilcd, cile, csi_tpg clocks.
+ See ../clocks/clock-bindings.txt for details.
+ - power-domains: Must include sor powergate node as csicil is in
+ SOR partition.
- epp: encoder pre-processor
@@ -309,13 +325,44 @@ Example:
reset-names = "mpe";
};
- vi {
- compatible = "nvidia,tegra20-vi";
- reg = <0x54080000 0x00040000>;
- interrupts = <0 69 0x04>;
- clocks = <&tegra_car TEGRA20_CLK_VI>;
- resets = <&tegra_car 100>;
- reset-names = "vi";
+ vi@54080000 {
+ compatible = "nvidia,tegra210-vi";
+ reg = <0x0 0x54080000 0x0 0x700>;
+ interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+ assigned-clocks = <&tegra_car TEGRA210_CLK_VI>;
+ assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
+
+ clocks = <&tegra_car TEGRA210_CLK_VI>;
+ power-domains = <&pd_venc>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ranges = <0x0 0x0 0x54080000 0x2000>;
+
+ csi@838 {
+ compatible = "nvidia,tegra210-csi";
+ reg = <0x838 0x1300>;
+ assigned-clocks = <&tegra_car TEGRA210_CLK_CILAB>,
+ <&tegra_car TEGRA210_CLK_CILCD>,
+ <&tegra_car TEGRA210_CLK_CILE>,
+ <&tegra_car TEGRA210_CLK_CSI_TPG>;
+ assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_P>,
+ <&tegra_car TEGRA210_CLK_PLL_P>,
+ <&tegra_car TEGRA210_CLK_PLL_P>;
+ assigned-clock-rates = <102000000>,
+ <102000000>,
+ <102000000>,
+ <972000000>;
+
+ clocks = <&tegra_car TEGRA210_CLK_CSI>,
+ <&tegra_car TEGRA210_CLK_CILAB>,
+ <&tegra_car TEGRA210_CLK_CILCD>,
+ <&tegra_car TEGRA210_CLK_CILE>,
+ <&tegra_car TEGRA210_CLK_CSI_TPG>;
+ clock-names = "csi", "cilab", "cilcd", "cile", "csi_tpg";
+ power-domains = <&pd_sor>;
+ };
};
epp {
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings
2020-04-22 6:18 ` [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings Sowjanya Komatineni
@ 2020-04-22 17:20 ` Laurent Pinchart
2020-04-22 17:26 ` Sowjanya Komatineni
0 siblings, 1 reply; 29+ messages in thread
From: Laurent Pinchart @ 2020-04-22 17:20 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: thierry.reding, jonathanh, frankc, hverkuil, sakari.ailus,
helen.koike, digetx, sboyd, linux-media, devicetree, linux-clk,
linux-tegra, linux-kernel
Hi Sowjanya,
Thank you for the patch.
On Tue, Apr 21, 2020 at 11:18:55PM -0700, Sowjanya Komatineni wrote:
> Tegra contains VI controller which can support up to 6 MIPI CSI
> camera sensors.
>
> Each Tegra CSI port from CSI unit can be one-to-one mapper to
> VI channel and can capture from an external camera sensor or
> from built-in test pattern generator.
>
> This patch adds dt-bindings for Tegra VI and CSI.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> .../display/tegra/nvidia,tegra20-host1x.txt | 73 ++++++++++++++++++----
> 1 file changed, 60 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> index 9999255..4731921 100644
> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> @@ -40,14 +40,30 @@ of the following host1x client modules:
>
> Required properties:
> - compatible: "nvidia,tegra<chip>-vi"
> - - reg: Physical base address and length of the controller's registers.
> + - reg: Physical base address and length of the controller registers.
> - interrupts: The interrupt outputs from the controller.
> - - clocks: Must contain one entry, for the module clock.
> + - clocks: clocks: Must contain one entry, for the module clock.
> See ../clocks/clock-bindings.txt for details.
> - - resets: Must contain an entry for each entry in reset-names.
> - See ../reset/reset.txt for details.
> - - reset-names: Must include the following entries:
> - - vi
> + - Tegra20/Tegra30/Tegra114/Tegra124:
> + - resets: Must contain an entry for each entry in reset-names.
> + See ../reset/reset.txt for details.
> + - reset-names: Must include the following entries:
> + - vi
> + - Tegra210:
> + - power-domains: Must include venc powergate node as vi is in VE partition.
> + - Tegra210 has CSI part of VI sharing same host interface and register space.
> + So, VI device node should have CSI child node.
> +
> + - csi: mipi csi interface to vi
> +
> + Required properties:
> + - compatible: "nvidia,tegra210-csi"
> + - reg: Physical base address offset to parent and length of the controller
> + registers.
> + - clocks: Must contain entries csi, cilab, cilcd, cile, csi_tpg clocks.
> + See ../clocks/clock-bindings.txt for details.
> + - power-domains: Must include sor powergate node as csicil is in
> + SOR partition.
A bit of a stupid question maybe, but why is this needed ? Can't the
driver that handles the vi DT node ("nvidia,tegra20-vi") handle all the
registers for all the sub-blocks ? Can't we move the clocks and power
domains from the CSI node to the VI node ?
Regardless of the answer to this question, I think this is missing port
nodes for the physical CSI-2 inputs, to connect them to sensors. I
haven't seen anywhere in this series how a CSI-2 sensor is linked to the
VI.
>
> - epp: encoder pre-processor
>
> @@ -309,13 +325,44 @@ Example:
> reset-names = "mpe";
> };
>
> - vi {
> - compatible = "nvidia,tegra20-vi";
> - reg = <0x54080000 0x00040000>;
> - interrupts = <0 69 0x04>;
> - clocks = <&tegra_car TEGRA20_CLK_VI>;
> - resets = <&tegra_car 100>;
> - reset-names = "vi";
> + vi@54080000 {
> + compatible = "nvidia,tegra210-vi";
> + reg = <0x0 0x54080000 0x0 0x700>;
> + interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> + assigned-clocks = <&tegra_car TEGRA210_CLK_VI>;
> + assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
> +
> + clocks = <&tegra_car TEGRA210_CLK_VI>;
> + power-domains = <&pd_venc>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + ranges = <0x0 0x0 0x54080000 0x2000>;
> +
> + csi@838 {
> + compatible = "nvidia,tegra210-csi";
> + reg = <0x838 0x1300>;
> + assigned-clocks = <&tegra_car TEGRA210_CLK_CILAB>,
> + <&tegra_car TEGRA210_CLK_CILCD>,
> + <&tegra_car TEGRA210_CLK_CILE>,
> + <&tegra_car TEGRA210_CLK_CSI_TPG>;
> + assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_P>,
> + <&tegra_car TEGRA210_CLK_PLL_P>,
> + <&tegra_car TEGRA210_CLK_PLL_P>;
> + assigned-clock-rates = <102000000>,
> + <102000000>,
> + <102000000>,
> + <972000000>;
> +
> + clocks = <&tegra_car TEGRA210_CLK_CSI>,
> + <&tegra_car TEGRA210_CLK_CILAB>,
> + <&tegra_car TEGRA210_CLK_CILCD>,
> + <&tegra_car TEGRA210_CLK_CILE>,
> + <&tegra_car TEGRA210_CLK_CSI_TPG>;
> + clock-names = "csi", "cilab", "cilcd", "cile", "csi_tpg";
> + power-domains = <&pd_sor>;
> + };
> };
>
> epp {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings
2020-04-22 17:20 ` Laurent Pinchart
@ 2020-04-22 17:26 ` Sowjanya Komatineni
2020-04-22 17:47 ` Laurent Pinchart
0 siblings, 1 reply; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 17:26 UTC (permalink / raw)
To: Laurent Pinchart
Cc: thierry.reding, jonathanh, frankc, hverkuil, sakari.ailus,
helen.koike, digetx, sboyd, linux-media, devicetree, linux-clk,
linux-tegra, linux-kernel
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1;
t=1587576320; bh=HfkzuR+vYVLOjA/839zIGQQg1loQfsQ+IO+/Ii0+hVQ=;
h=X-PGP-Universal:Subject:To:CC:References:From:Message-ID:Date:
User-Agent:MIME-Version:In-Reply-To:X-Originating-IP:
X-ClientProxiedBy:Content-Type:Content-Transfer-Encoding:
Content-Language;
b=mXG4pQ46mtw3JTRdiTAfbKIo8ocWIwYbDoR2GlvGZNfM++w8WWv2futApPFpQPDsn
DYKCKfLjQS6QMi9iado5yxDppXLzKNPSAh79WiOtZkDnxnYesOfrbPrR4NvX7uV1Mh
j8yBi3WYowY2ACc/ouLfLAY4GLLXdEDKQeIi7ztdEUZlqCHH9JdQYzt55yeXTSG7Ju
EdBSajJgI0RbyjAhZs9YwJ8/Sk5TpOrby6kmzk2gCzvkCYPp98U69bLnqRccI/uf4e
W6Urof3IfLm6MYH3rDE3nrQmGRMu713Us4o9WX+atKkaCQ+pZUZklqfm9KrCTAdrIL
Ba0krgo3iyjDQ==
On 4/22/20 10:20 AM, Laurent Pinchart wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Sowjanya,
>
> Thank you for the patch.
>
> On Tue, Apr 21, 2020 at 11:18:55PM -0700, Sowjanya Komatineni wrote:
>> Tegra contains VI controller which can support up to 6 MIPI CSI
>> camera sensors.
>>
>> Each Tegra CSI port from CSI unit can be one-to-one mapper to
>> VI channel and can capture from an external camera sensor or
>> from built-in test pattern generator.
>>
>> This patch adds dt-bindings for Tegra VI and CSI.
>>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> .../display/tegra/nvidia,tegra20-host1x.txt | 73 ++++++++++++++++++----
>> 1 file changed, 60 insertions(+), 13 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> index 9999255..4731921 100644
>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> @@ -40,14 +40,30 @@ of the following host1x client modules:
>>
>> Required properties:
>> - compatible: "nvidia,tegra<chip>-vi"
>> - - reg: Physical base address and length of the controller's registers.
>> + - reg: Physical base address and length of the controller registers.
>> - interrupts: The interrupt outputs from the controller.
>> - - clocks: Must contain one entry, for the module clock.
>> + - clocks: clocks: Must contain one entry, for the module clock.
>> See ../clocks/clock-bindings.txt for details.
>> - - resets: Must contain an entry for each entry in reset-names.
>> - See ../reset/reset.txt for details.
>> - - reset-names: Must include the following entries:
>> - - vi
>> + - Tegra20/Tegra30/Tegra114/Tegra124:
>> + - resets: Must contain an entry for each entry in reset-names.
>> + See ../reset/reset.txt for details.
>> + - reset-names: Must include the following entries:
>> + - vi
>> + - Tegra210:
>> + - power-domains: Must include venc powergate node as vi is in VE partition.
>> + - Tegra210 has CSI part of VI sharing same host interface and register space.
>> + So, VI device node should have CSI child node.
>> +
>> + - csi: mipi csi interface to vi
>> +
>> + Required properties:
>> + - compatible: "nvidia,tegra210-csi"
>> + - reg: Physical base address offset to parent and length of the controller
>> + registers.
>> + - clocks: Must contain entries csi, cilab, cilcd, cile, csi_tpg clocks.
>> + See ../clocks/clock-bindings.txt for details.
>> + - power-domains: Must include sor powergate node as csicil is in
>> + SOR partition.
> A bit of a stupid question maybe, but why is this needed ? Can't the
> driver that handles the vi DT node ("nvidia,tegra20-vi") handle all the
> registers for all the sub-blocks ? Can't we move the clocks and power
> domains from the CSI node to the VI node ?
CSI is separate device driver and VI is separate device driver.
For T210, CSI shares register space under VI but for later Tegras its
separate register space.
So CSI and VI drivers are separate with their corresponding clocks and
power domains in their nodes.
>
> Regardless of the answer to this question, I think this is missing port
> nodes for the physical CSI-2 inputs, to connect them to sensors. I
> haven't seen anywhere in this series how a CSI-2 sensor is linked to the
> VI.
This patch series is only for Tegra internal TPG and tegra video driver
creates hard media links between CSI and VI,
Sensor support will be in Series-2 where port nodes will be used for
real sensor <-> csi <-> vi endpoints
>
>> - epp: encoder pre-processor
>>
>> @@ -309,13 +325,44 @@ Example:
>> reset-names = "mpe";
>> };
>>
>> - vi {
>> - compatible = "nvidia,tegra20-vi";
>> - reg = <0x54080000 0x00040000>;
>> - interrupts = <0 69 0x04>;
>> - clocks = <&tegra_car TEGRA20_CLK_VI>;
>> - resets = <&tegra_car 100>;
>> - reset-names = "vi";
>> + vi@54080000 {
>> + compatible = "nvidia,tegra210-vi";
>> + reg = <0x0 0x54080000 0x0 0x700>;
>> + interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
>> + assigned-clocks = <&tegra_car TEGRA210_CLK_VI>;
>> + assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
>> +
>> + clocks = <&tegra_car TEGRA210_CLK_VI>;
>> + power-domains = <&pd_venc>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + ranges = <0x0 0x0 0x54080000 0x2000>;
>> +
>> + csi@838 {
>> + compatible = "nvidia,tegra210-csi";
>> + reg = <0x838 0x1300>;
>> + assigned-clocks = <&tegra_car TEGRA210_CLK_CILAB>,
>> + <&tegra_car TEGRA210_CLK_CILCD>,
>> + <&tegra_car TEGRA210_CLK_CILE>,
>> + <&tegra_car TEGRA210_CLK_CSI_TPG>;
>> + assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_P>,
>> + <&tegra_car TEGRA210_CLK_PLL_P>,
>> + <&tegra_car TEGRA210_CLK_PLL_P>;
>> + assigned-clock-rates = <102000000>,
>> + <102000000>,
>> + <102000000>,
>> + <972000000>;
>> +
>> + clocks = <&tegra_car TEGRA210_CLK_CSI>,
>> + <&tegra_car TEGRA210_CLK_CILAB>,
>> + <&tegra_car TEGRA210_CLK_CILCD>,
>> + <&tegra_car TEGRA210_CLK_CILE>,
>> + <&tegra_car TEGRA210_CLK_CSI_TPG>;
>> + clock-names = "csi", "cilab", "cilcd", "cile", "csi_tpg";
>> + power-domains = <&pd_sor>;
>> + };
>> };
>>
>> epp {
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings
2020-04-22 17:26 ` Sowjanya Komatineni
@ 2020-04-22 17:47 ` Laurent Pinchart
2020-04-22 17:57 ` Sowjanya Komatineni
0 siblings, 1 reply; 29+ messages in thread
From: Laurent Pinchart @ 2020-04-22 17:47 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: thierry.reding, jonathanh, frankc, hverkuil, sakari.ailus,
helen.koike, digetx, sboyd, linux-media, devicetree, linux-clk,
linux-tegra, linux-kernel
Hi Sowjanya,
On Wed, Apr 22, 2020 at 10:26:20AM -0700, Sowjanya Komatineni wrote:
> On 4/22/20 10:20 AM, Laurent Pinchart wrote:
> > On Tue, Apr 21, 2020 at 11:18:55PM -0700, Sowjanya Komatineni wrote:
> >> Tegra contains VI controller which can support up to 6 MIPI CSI
> >> camera sensors.
> >>
> >> Each Tegra CSI port from CSI unit can be one-to-one mapper to
> >> VI channel and can capture from an external camera sensor or
> >> from built-in test pattern generator.
> >>
> >> This patch adds dt-bindings for Tegra VI and CSI.
> >>
> >> Acked-by: Thierry Reding <treding@nvidia.com>
> >> Reviewed-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> >> ---
> >> .../display/tegra/nvidia,tegra20-host1x.txt | 73 ++++++++++++++++++----
> >> 1 file changed, 60 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> >> index 9999255..4731921 100644
> >> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> >> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> >> @@ -40,14 +40,30 @@ of the following host1x client modules:
> >>
> >> Required properties:
> >> - compatible: "nvidia,tegra<chip>-vi"
> >> - - reg: Physical base address and length of the controller's registers.
> >> + - reg: Physical base address and length of the controller registers.
> >> - interrupts: The interrupt outputs from the controller.
> >> - - clocks: Must contain one entry, for the module clock.
> >> + - clocks: clocks: Must contain one entry, for the module clock.
> >> See ../clocks/clock-bindings.txt for details.
> >> - - resets: Must contain an entry for each entry in reset-names.
> >> - See ../reset/reset.txt for details.
> >> - - reset-names: Must include the following entries:
> >> - - vi
> >> + - Tegra20/Tegra30/Tegra114/Tegra124:
> >> + - resets: Must contain an entry for each entry in reset-names.
> >> + See ../reset/reset.txt for details.
> >> + - reset-names: Must include the following entries:
> >> + - vi
> >> + - Tegra210:
> >> + - power-domains: Must include venc powergate node as vi is in VE partition.
> >> + - Tegra210 has CSI part of VI sharing same host interface and register space.
> >> + So, VI device node should have CSI child node.
> >> +
> >> + - csi: mipi csi interface to vi
> >> +
> >> + Required properties:
> >> + - compatible: "nvidia,tegra210-csi"
> >> + - reg: Physical base address offset to parent and length of the controller
> >> + registers.
> >> + - clocks: Must contain entries csi, cilab, cilcd, cile, csi_tpg clocks.
> >> + See ../clocks/clock-bindings.txt for details.
> >> + - power-domains: Must include sor powergate node as csicil is in
> >> + SOR partition.
> >
> > A bit of a stupid question maybe, but why is this needed ? Can't the
> > driver that handles the vi DT node ("nvidia,tegra20-vi") handle all the
> > registers for all the sub-blocks ? Can't we move the clocks and power
> > domains from the CSI node to the VI node ?
>
> CSI is separate device driver and VI is separate device driver.
That's fine, but that's a software design decision, it should not affect
the DT bindings. It's possible (even if I don't necessarily recommend
doing so) to create a platform device manually in the VI driver to get
it bound to the CSI driver. In any case DT should describe the system
architecture and shouldn't be influenced by hardware design.
> For T210, CSI shares register space under VI but for later Tegras its
> separate register space.
This is useful information. How about interrupts though ? You don't
specify any interrupt line here, how are the CSI interrutps handled ?
> So CSI and VI drivers are separate with their corresponding clocks and
> power domains in their nodes.
>
> > Regardless of the answer to this question, I think this is missing port
> > nodes for the physical CSI-2 inputs, to connect them to sensors. I
> > haven't seen anywhere in this series how a CSI-2 sensor is linked to the
> > VI.
>
> This patch series is only for Tegra internal TPG and tegra video driver
> creates hard media links between CSI and VI,
Could you share the output of media-ctl --print-dot to show how this
looks like ?
> Sensor support will be in Series-2 where port nodes will be used for
> real sensor <-> csi <-> vi endpoints
>
> >> - epp: encoder pre-processor
> >>
> >> @@ -309,13 +325,44 @@ Example:
> >> reset-names = "mpe";
> >> };
> >>
> >> - vi {
> >> - compatible = "nvidia,tegra20-vi";
> >> - reg = <0x54080000 0x00040000>;
> >> - interrupts = <0 69 0x04>;
> >> - clocks = <&tegra_car TEGRA20_CLK_VI>;
> >> - resets = <&tegra_car 100>;
> >> - reset-names = "vi";
> >> + vi@54080000 {
> >> + compatible = "nvidia,tegra210-vi";
> >> + reg = <0x0 0x54080000 0x0 0x700>;
> >> + interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> >> + assigned-clocks = <&tegra_car TEGRA210_CLK_VI>;
> >> + assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
> >> +
> >> + clocks = <&tegra_car TEGRA210_CLK_VI>;
> >> + power-domains = <&pd_venc>;
> >> +
> >> + #address-cells = <1>;
> >> + #size-cells = <1>;
> >> +
> >> + ranges = <0x0 0x0 0x54080000 0x2000>;
> >> +
> >> + csi@838 {
> >> + compatible = "nvidia,tegra210-csi";
> >> + reg = <0x838 0x1300>;
> >> + assigned-clocks = <&tegra_car TEGRA210_CLK_CILAB>,
> >> + <&tegra_car TEGRA210_CLK_CILCD>,
> >> + <&tegra_car TEGRA210_CLK_CILE>,
> >> + <&tegra_car TEGRA210_CLK_CSI_TPG>;
> >> + assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_P>,
> >> + <&tegra_car TEGRA210_CLK_PLL_P>,
> >> + <&tegra_car TEGRA210_CLK_PLL_P>;
> >> + assigned-clock-rates = <102000000>,
> >> + <102000000>,
> >> + <102000000>,
> >> + <972000000>;
> >> +
> >> + clocks = <&tegra_car TEGRA210_CLK_CSI>,
> >> + <&tegra_car TEGRA210_CLK_CILAB>,
> >> + <&tegra_car TEGRA210_CLK_CILCD>,
> >> + <&tegra_car TEGRA210_CLK_CILE>,
> >> + <&tegra_car TEGRA210_CLK_CSI_TPG>;
> >> + clock-names = "csi", "cilab", "cilcd", "cile", "csi_tpg";
> >> + power-domains = <&pd_sor>;
> >> + };
> >> };
> >>
> >> epp {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings
2020-04-22 17:47 ` Laurent Pinchart
@ 2020-04-22 17:57 ` Sowjanya Komatineni
0 siblings, 0 replies; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 17:57 UTC (permalink / raw)
To: Laurent Pinchart
Cc: thierry.reding, jonathanh, frankc, hverkuil, sakari.ailus,
helen.koike, digetx, sboyd, linux-media, devicetree, linux-clk,
linux-tegra, linux-kernel
On 4/22/20 10:47 AM, Laurent Pinchart wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Sowjanya,
>
> On Wed, Apr 22, 2020 at 10:26:20AM -0700, Sowjanya Komatineni wrote:
>> On 4/22/20 10:20 AM, Laurent Pinchart wrote:
>>> On Tue, Apr 21, 2020 at 11:18:55PM -0700, Sowjanya Komatineni wrote:
>>>> Tegra contains VI controller which can support up to 6 MIPI CSI
>>>> camera sensors.
>>>>
>>>> Each Tegra CSI port from CSI unit can be one-to-one mapper to
>>>> VI channel and can capture from an external camera sensor or
>>>> from built-in test pattern generator.
>>>>
>>>> This patch adds dt-bindings for Tegra VI and CSI.
>>>>
>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> .../display/tegra/nvidia,tegra20-host1x.txt | 73 ++++++++++++++++++----
>>>> 1 file changed, 60 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>>> index 9999255..4731921 100644
>>>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>>> @@ -40,14 +40,30 @@ of the following host1x client modules:
>>>>
>>>> Required properties:
>>>> - compatible: "nvidia,tegra<chip>-vi"
>>>> - - reg: Physical base address and length of the controller's registers.
>>>> + - reg: Physical base address and length of the controller registers.
>>>> - interrupts: The interrupt outputs from the controller.
>>>> - - clocks: Must contain one entry, for the module clock.
>>>> + - clocks: clocks: Must contain one entry, for the module clock.
>>>> See ../clocks/clock-bindings.txt for details.
>>>> - - resets: Must contain an entry for each entry in reset-names.
>>>> - See ../reset/reset.txt for details.
>>>> - - reset-names: Must include the following entries:
>>>> - - vi
>>>> + - Tegra20/Tegra30/Tegra114/Tegra124:
>>>> + - resets: Must contain an entry for each entry in reset-names.
>>>> + See ../reset/reset.txt for details.
>>>> + - reset-names: Must include the following entries:
>>>> + - vi
>>>> + - Tegra210:
>>>> + - power-domains: Must include venc powergate node as vi is in VE partition.
>>>> + - Tegra210 has CSI part of VI sharing same host interface and register space.
>>>> + So, VI device node should have CSI child node.
>>>> +
>>>> + - csi: mipi csi interface to vi
>>>> +
>>>> + Required properties:
>>>> + - compatible: "nvidia,tegra210-csi"
>>>> + - reg: Physical base address offset to parent and length of the controller
>>>> + registers.
>>>> + - clocks: Must contain entries csi, cilab, cilcd, cile, csi_tpg clocks.
>>>> + See ../clocks/clock-bindings.txt for details.
>>>> + - power-domains: Must include sor powergate node as csicil is in
>>>> + SOR partition.
>>> A bit of a stupid question maybe, but why is this needed ? Can't the
>>> driver that handles the vi DT node ("nvidia,tegra20-vi") handle all the
>>> registers for all the sub-blocks ? Can't we move the clocks and power
>>> domains from the CSI node to the VI node ?
>> CSI is separate device driver and VI is separate device driver.
> That's fine, but that's a software design decision, it should not affect
> the DT bindings. It's possible (even if I don't necessarily recommend
> doing so) to create a platform device manually in the VI driver to get
> it bound to the CSI driver. In any case DT should describe the system
> architecture and shouldn't be influenced by hardware design.
CSI is a separate device for Tegra186 onwards and VI/CSI is common driver.
DT bindings reflects the chip design. Also clocks and power domain for
CSI and VI are different and CSI is separate device.
>
>> For T210, CSI shares register space under VI but for later Tegras its
>> separate register space.
> This is useful information. How about interrupts though ? You don't
> specify any interrupt line here, how are the CSI interrutps handled ?
All frame events are through syncpoint HW like mentioned in earlier
feedbacks.
This driver synchronizes capture events through host1x syncpt
>
>> So CSI and VI drivers are separate with their corresponding clocks and
>> power domains in their nodes.
>>
>>> Regardless of the answer to this question, I think this is missing port
>>> nodes for the physical CSI-2 inputs, to connect them to sensors. I
>>> haven't seen anywhere in this series how a CSI-2 sensor is linked to the
>>> VI.
>> This patch series is only for Tegra internal TPG and tegra video driver
>> creates hard media links between CSI and VI,
> Could you share the output of media-ctl --print-dot to show how this
> looks like ?
digraph board {
rankdir=TB
n00000001 [label="{{} | tpg-0\n | {<port0> 0}}", shape=Mrecord,
style=filled, fillcolor=green]
n00000001:port0 -> n00000003
n00000003 [label="54080000.vi-output-0\n/dev/video0",
shape=box, style=filled, fillcolor=yellow]
n00000009 [label="{{} | tpg-1\n | {<port0> 0}}", shape=Mrecord,
style=filled, fillcolor=green]
n00000009:port0 -> n0000000b
n0000000b [label="54080000.vi-output-1\n/dev/video1",
shape=box, style=filled, fillcolor=yellow]
n00000011 [label="{{} | tpg-2\n | {<port0> 0}}", shape=Mrecord,
style=filled, fillcolor=green]
n00000011:port0 -> n00000013
n00000013 [label="54080000.vi-output-2\n/dev/video2",
shape=box, style=filled, fillcolor=yellow]
n00000019 [label="{{} | tpg-3\n | {<port0> 0}}", shape=Mrecord,
style=filled, fillcolor=green]
n00000019:port0 -> n0000001b
n0000001b [label="54080000.vi-output-3\n/dev/video3",
shape=box, style=filled, fillcolor=yellow]
n00000021 [label="{{} | tpg-4\n | {<port0> 0}}", shape=Mrecord,
style=filled, fillcolor=green]
n00000021:port0 -> n00000023
n00000023 [label="54080000.vi-output-4\n/dev/video4",
shape=box, style=filled, fillcolor=yellow]
n00000029 [label="{{} | tpg-5\n | {<port0> 0}}", shape=Mrecord,
style=filled, fillcolor=green]
n00000029:port0 -> n0000002b
n0000002b [label="54080000.vi-output-5\n/dev/video5",
shape=box, style=filled, fillcolor=yellow]
}
>
>> Sensor support will be in Series-2 where port nodes will be used for
>> real sensor <-> csi <-> vi endpoints
>>
>>>> - epp: encoder pre-processor
>>>>
>>>> @@ -309,13 +325,44 @@ Example:
>>>> reset-names = "mpe";
>>>> };
>>>>
>>>> - vi {
>>>> - compatible = "nvidia,tegra20-vi";
>>>> - reg = <0x54080000 0x00040000>;
>>>> - interrupts = <0 69 0x04>;
>>>> - clocks = <&tegra_car TEGRA20_CLK_VI>;
>>>> - resets = <&tegra_car 100>;
>>>> - reset-names = "vi";
>>>> + vi@54080000 {
>>>> + compatible = "nvidia,tegra210-vi";
>>>> + reg = <0x0 0x54080000 0x0 0x700>;
>>>> + interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
>>>> + assigned-clocks = <&tegra_car TEGRA210_CLK_VI>;
>>>> + assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
>>>> +
>>>> + clocks = <&tegra_car TEGRA210_CLK_VI>;
>>>> + power-domains = <&pd_venc>;
>>>> +
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>> +
>>>> + ranges = <0x0 0x0 0x54080000 0x2000>;
>>>> +
>>>> + csi@838 {
>>>> + compatible = "nvidia,tegra210-csi";
>>>> + reg = <0x838 0x1300>;
>>>> + assigned-clocks = <&tegra_car TEGRA210_CLK_CILAB>,
>>>> + <&tegra_car TEGRA210_CLK_CILCD>,
>>>> + <&tegra_car TEGRA210_CLK_CILE>,
>>>> + <&tegra_car TEGRA210_CLK_CSI_TPG>;
>>>> + assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_P>,
>>>> + <&tegra_car TEGRA210_CLK_PLL_P>,
>>>> + <&tegra_car TEGRA210_CLK_PLL_P>;
>>>> + assigned-clock-rates = <102000000>,
>>>> + <102000000>,
>>>> + <102000000>,
>>>> + <972000000>;
>>>> +
>>>> + clocks = <&tegra_car TEGRA210_CLK_CSI>,
>>>> + <&tegra_car TEGRA210_CLK_CILAB>,
>>>> + <&tegra_car TEGRA210_CLK_CILCD>,
>>>> + <&tegra_car TEGRA210_CLK_CILE>,
>>>> + <&tegra_car TEGRA210_CLK_CSI_TPG>;
>>>> + clock-names = "csi", "cilab", "cilcd", "cile", "csi_tpg";
>>>> + power-domains = <&pd_sor>;
>>>> + };
>>>> };
>>>>
>>>> epp {
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 29+ messages in thread
* [RFC PATCH v9 7/9] MAINTAINERS: Add Tegra Video driver section
2020-04-22 6:18 [RFC PATCH v9 0/9] Add Tegra driver for video capture Sowjanya Komatineni
` (4 preceding siblings ...)
2020-04-22 6:18 ` [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings Sowjanya Komatineni
@ 2020-04-22 6:18 ` Sowjanya Komatineni
2020-04-22 6:18 ` [RFC PATCH v9 8/9] dt-bindings: reset: Add ID for Tegra210 VI reset Sowjanya Komatineni
` (2 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 6:18 UTC (permalink / raw)
To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
Add maintainers and mailing list entries to Tegra Video driver section.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
MAINTAINERS | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5872577..47be371 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16608,6 +16608,16 @@ M: Laxman Dewangan <ldewangan@nvidia.com>
S: Supported
F: drivers/spi/spi-tegra*
+TEGRA VIDEO DRIVER
+M: Thierry Reding <thierry.reding@gmail.com>
+M: Jonathan Hunter <jonathanh@nvidia.com>
+M: Sowjanya Komatineni <skomatineni@nvidia.com>
+L: linux-media@vger.kernel.org
+L: linux-tegra@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
+F: drivers/staging/media/tegra/
+
TEGRA XUSB PADCTL DRIVER
M: JC Kuo <jckuo@nvidia.com>
S: Supported
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [RFC PATCH v9 8/9] dt-bindings: reset: Add ID for Tegra210 VI reset
2020-04-22 6:18 [RFC PATCH v9 0/9] Add Tegra driver for video capture Sowjanya Komatineni
` (5 preceding siblings ...)
2020-04-22 6:18 ` [RFC PATCH v9 7/9] MAINTAINERS: Add Tegra Video driver section Sowjanya Komatineni
@ 2020-04-22 6:18 ` Sowjanya Komatineni
2020-04-22 6:18 ` [RFC PATCH v9 9/9] arm64: tegra: Add Tegra VI CSI support in device tree Sowjanya Komatineni
[not found] ` <1587536339-4030-7-git-send-email-skomatineni@nvidia.com>
8 siblings, 0 replies; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 6:18 UTC (permalink / raw)
To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
This patch adds ID for Tegra210 VI controller reset to use with
device tree.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
include/dt-bindings/reset/tegra210-car.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/dt-bindings/reset/tegra210-car.h b/include/dt-bindings/reset/tegra210-car.h
index 9dc84ec..8755946 100644
--- a/include/dt-bindings/reset/tegra210-car.h
+++ b/include/dt-bindings/reset/tegra210-car.h
@@ -10,5 +10,6 @@
#define TEGRA210_RESET(x) (7 * 32 + (x))
#define TEGRA210_RST_DFLL_DVCO TEGRA210_RESET(0)
#define TEGRA210_RST_ADSP TEGRA210_RESET(1)
+#define TEGRA210_RST_VI 20
#endif /* _DT_BINDINGS_RESET_TEGRA210_CAR_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [RFC PATCH v9 9/9] arm64: tegra: Add Tegra VI CSI support in device tree
2020-04-22 6:18 [RFC PATCH v9 0/9] Add Tegra driver for video capture Sowjanya Komatineni
` (6 preceding siblings ...)
2020-04-22 6:18 ` [RFC PATCH v9 8/9] dt-bindings: reset: Add ID for Tegra210 VI reset Sowjanya Komatineni
@ 2020-04-22 6:18 ` Sowjanya Komatineni
[not found] ` <1587536339-4030-7-git-send-email-skomatineni@nvidia.com>
8 siblings, 0 replies; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-22 6:18 UTC (permalink / raw)
To: skomatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
Tegra210 contains VI controller for video input capture from MIPI
CSI camera sensors and also supports built-in test pattern generator.
CSI ports can be one-to-one mapped to VI channels for capturing from
an external sensor or from built-in test pattern generator.
This patch adds support for VI and CSI and enables them in Tegra210
device tree.
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi | 10 ++++++
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 46 +++++++++++++++++++++++++-
2 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
index 313a4c2..b57d837 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
@@ -14,6 +14,16 @@
status = "okay";
};
+ vi@54080000 {
+ status = "okay";
+
+ avdd-dsi-csi-supply = <&vdd_dsi_csi>;
+
+ csi@838 {
+ status = "okay";
+ };
+ };
+
sor@54580000 {
status = "okay";
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 5b1dfd8..cad42a7 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -137,9 +137,44 @@
vi@54080000 {
compatible = "nvidia,tegra210-vi";
- reg = <0x0 0x54080000 0x0 0x00040000>;
+ reg = <0x0 0x54080000 0x0 0x700>;
interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
status = "disabled";
+ assigned-clocks = <&tegra_car TEGRA210_CLK_VI>;
+ assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_C4_OUT0>;
+
+ clocks = <&tegra_car TEGRA210_CLK_VI>;
+ power-domains = <&pd_venc>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ranges = <0x0 0x0 0x54080000 0x2000>;
+
+ csi@838 {
+ compatible = "nvidia,tegra210-csi";
+ reg = <0x838 0x1300>;
+ status = "disabled";
+ assigned-clocks = <&tegra_car TEGRA210_CLK_CILAB>,
+ <&tegra_car TEGRA210_CLK_CILCD>,
+ <&tegra_car TEGRA210_CLK_CILE>,
+ <&tegra_car TEGRA210_CLK_CSI_TPG>;
+ assigned-clock-parents = <&tegra_car TEGRA210_CLK_PLL_P>,
+ <&tegra_car TEGRA210_CLK_PLL_P>,
+ <&tegra_car TEGRA210_CLK_PLL_P>;
+ assigned-clock-rates = <102000000>,
+ <102000000>,
+ <102000000>,
+ <972000000>;
+
+ clocks = <&tegra_car TEGRA210_CLK_CSI>,
+ <&tegra_car TEGRA210_CLK_CILAB>,
+ <&tegra_car TEGRA210_CLK_CILCD>,
+ <&tegra_car TEGRA210_CLK_CILE>,
+ <&tegra_car TEGRA210_CLK_CSI_TPG>;
+ clock-names = "csi", "cilab", "cilcd", "cile", "csi_tpg";
+ power-domains = <&pd_sor>;
+ };
};
tsec@54100000 {
@@ -839,6 +874,15 @@
reset-names = "vic";
#power-domain-cells = <0>;
};
+
+ pd_venc: venc {
+ clocks = <&tegra_car TEGRA210_CLK_VI>,
+ <&tegra_car TEGRA210_CLK_CSI>;
+ resets = <&mc TEGRA210_MC_RESET_VI>,
+ <&tegra_car TEGRA210_RST_VI>,
+ <&tegra_car TEGRA210_CLK_CSI>;
+ #power-domain-cells = <0>;
+ };
};
sdmmc1_3v3: sdmmc1-3v3 {
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread[parent not found: <1587536339-4030-7-git-send-email-skomatineni@nvidia.com>]
* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
[not found] ` <1587536339-4030-7-git-send-email-skomatineni@nvidia.com>
@ 2020-04-23 7:48 ` Hans Verkuil
2020-04-23 16:50 ` Sowjanya Komatineni
2020-04-23 23:16 ` Dmitry Osipenko
1 sibling, 1 reply; 29+ messages in thread
From: Hans Verkuil @ 2020-04-23 7:48 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, frankc,
sakari.ailus, helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
On 22/04/2020 08:18, Sowjanya Komatineni wrote:
> Tegra210 contains a powerful Video Input (VI) hardware controller
> which can support up to 6 MIPI CSI camera sensors.
>
> Each Tegra CSI port can be one-to-one mapped to VI channel and can
> capture from an external camera sensor connected to CSI or from
> built-in test pattern generator.
>
> Tegra210 supports built-in test pattern generator from CSI to VI.
>
> This patch adds a v4l2 capture driver with media interface for
> Tegra210 built-in CSI to VI test pattern generator.
>
> This patch includes TPG support only and all the video pipeline
> configuration happens through the video device node.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/staging/media/Kconfig | 2 +
> drivers/staging/media/Makefile | 1 +
> drivers/staging/media/tegra/Kconfig | 13 +
> drivers/staging/media/tegra/Makefile | 8 +
> drivers/staging/media/tegra/TODO | 10 +
> drivers/staging/media/tegra/common.h | 262 ++++++++
> drivers/staging/media/tegra/csi.c | 606 +++++++++++++++++
> drivers/staging/media/tegra/csi.h | 149 +++++
> drivers/staging/media/tegra/tegra210.c | 709 ++++++++++++++++++++
> drivers/staging/media/tegra/tegra210.h | 190 ++++++
> drivers/staging/media/tegra/vi.c | 1132 ++++++++++++++++++++++++++++++++
> drivers/staging/media/tegra/vi.h | 83 +++
> drivers/staging/media/tegra/video.c | 153 +++++
> drivers/staging/media/tegra/video.h | 34 +
> 14 files changed, 3352 insertions(+)
> create mode 100644 drivers/staging/media/tegra/Kconfig
> create mode 100644 drivers/staging/media/tegra/Makefile
> create mode 100644 drivers/staging/media/tegra/TODO
> create mode 100644 drivers/staging/media/tegra/common.h
> create mode 100644 drivers/staging/media/tegra/csi.c
> create mode 100644 drivers/staging/media/tegra/csi.h
> create mode 100644 drivers/staging/media/tegra/tegra210.c
> create mode 100644 drivers/staging/media/tegra/tegra210.h
> create mode 100644 drivers/staging/media/tegra/vi.c
> create mode 100644 drivers/staging/media/tegra/vi.h
> create mode 100644 drivers/staging/media/tegra/video.c
> create mode 100644 drivers/staging/media/tegra/video.h
With 'make menuconfig' I get this:
scripts/kconfig/mconf Kconfig
WARNING: unmet direct dependencies detected for TEGRA_HOST1X
Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM && COMPILE_TEST [=y])
Selected by [y]:
- VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] && MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
This is an x86_64 build with COMPILE_TEST set. I can provide my full .config if you need it.
CONFIG_TEGRA_HOST1X=y
CONFIG_VIDEO_TEGRA=y
Regards,
Hans
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 7:48 ` [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver Hans Verkuil
@ 2020-04-23 16:50 ` Sowjanya Komatineni
2020-04-23 22:55 ` Dmitry Osipenko
0 siblings, 1 reply; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-23 16:50 UTC (permalink / raw)
To: Hans Verkuil, thierry.reding, jonathanh, frankc, sakari.ailus,
helen.koike
Cc: digetx, sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
On 4/23/20 12:48 AM, Hans Verkuil wrote:
> External email: Use caution opening links or attachments
>
>
> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>> Tegra210 contains a powerful Video Input (VI) hardware controller
>> which can support up to 6 MIPI CSI camera sensors.
>>
>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>> capture from an external camera sensor connected to CSI or from
>> built-in test pattern generator.
>>
>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>
>> This patch adds a v4l2 capture driver with media interface for
>> Tegra210 built-in CSI to VI test pattern generator.
>>
>> This patch includes TPG support only and all the video pipeline
>> configuration happens through the video device node.
>>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/staging/media/Kconfig | 2 +
>> drivers/staging/media/Makefile | 1 +
>> drivers/staging/media/tegra/Kconfig | 13 +
>> drivers/staging/media/tegra/Makefile | 8 +
>> drivers/staging/media/tegra/TODO | 10 +
>> drivers/staging/media/tegra/common.h | 262 ++++++++
>> drivers/staging/media/tegra/csi.c | 606 +++++++++++++++++
>> drivers/staging/media/tegra/csi.h | 149 +++++
>> drivers/staging/media/tegra/tegra210.c | 709 ++++++++++++++++++++
>> drivers/staging/media/tegra/tegra210.h | 190 ++++++
>> drivers/staging/media/tegra/vi.c | 1132 ++++++++++++++++++++++++++++++++
>> drivers/staging/media/tegra/vi.h | 83 +++
>> drivers/staging/media/tegra/video.c | 153 +++++
>> drivers/staging/media/tegra/video.h | 34 +
>> 14 files changed, 3352 insertions(+)
>> create mode 100644 drivers/staging/media/tegra/Kconfig
>> create mode 100644 drivers/staging/media/tegra/Makefile
>> create mode 100644 drivers/staging/media/tegra/TODO
>> create mode 100644 drivers/staging/media/tegra/common.h
>> create mode 100644 drivers/staging/media/tegra/csi.c
>> create mode 100644 drivers/staging/media/tegra/csi.h
>> create mode 100644 drivers/staging/media/tegra/tegra210.c
>> create mode 100644 drivers/staging/media/tegra/tegra210.h
>> create mode 100644 drivers/staging/media/tegra/vi.c
>> create mode 100644 drivers/staging/media/tegra/vi.h
>> create mode 100644 drivers/staging/media/tegra/video.c
>> create mode 100644 drivers/staging/media/tegra/video.h
> With 'make menuconfig' I get this:
>
> scripts/kconfig/mconf Kconfig
>
> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
> Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM && COMPILE_TEST [=y])
> Selected by [y]:
> - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] && MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>
> This is an x86_64 build with COMPILE_TEST set. I can provide my full .config if you need it.
>
> CONFIG_TEGRA_HOST1X=y
> CONFIG_VIDEO_TEGRA=y
>
> Regards,
>
> Hans
Hi Hans,
In v7, changed Kconfig to remove ARM. But looks like we should limit
TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on
x86_64 shows above warning.
We should limit compile to ARM for CONFIG_VIDEO_TEGRA.
Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST
like I had in previous version. Sorry about this.
Also, I see some changes went into latest linux-next staging media
Kconfig, So, will have my patches on top of today's linux-next.
Thanks
Sowjanya
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 16:50 ` Sowjanya Komatineni
@ 2020-04-23 22:55 ` Dmitry Osipenko
2020-04-23 22:59 ` Sowjanya Komatineni
0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Osipenko @ 2020-04-23 22:55 UTC (permalink / raw)
To: Sowjanya Komatineni, Hans Verkuil, thierry.reding, jonathanh,
frankc, sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
23.04.2020 19:50, Sowjanya Komatineni пишет:
>
> On 4/23/20 12:48 AM, Hans Verkuil wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>>> Tegra210 contains a powerful Video Input (VI) hardware controller
>>> which can support up to 6 MIPI CSI camera sensors.
>>>
>>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>>> capture from an external camera sensor connected to CSI or from
>>> built-in test pattern generator.
>>>
>>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>>
>>> This patch adds a v4l2 capture driver with media interface for
>>> Tegra210 built-in CSI to VI test pattern generator.
>>>
>>> This patch includes TPG support only and all the video pipeline
>>> configuration happens through the video device node.
>>>
>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>> ---
>>> drivers/staging/media/Kconfig | 2 +
>>> drivers/staging/media/Makefile | 1 +
>>> drivers/staging/media/tegra/Kconfig | 13 +
>>> drivers/staging/media/tegra/Makefile | 8 +
>>> drivers/staging/media/tegra/TODO | 10 +
>>> drivers/staging/media/tegra/common.h | 262 ++++++++
>>> drivers/staging/media/tegra/csi.c | 606 +++++++++++++++++
>>> drivers/staging/media/tegra/csi.h | 149 +++++
>>> drivers/staging/media/tegra/tegra210.c | 709 ++++++++++++++++++++
>>> drivers/staging/media/tegra/tegra210.h | 190 ++++++
>>> drivers/staging/media/tegra/vi.c | 1132
>>> ++++++++++++++++++++++++++++++++
>>> drivers/staging/media/tegra/vi.h | 83 +++
>>> drivers/staging/media/tegra/video.c | 153 +++++
>>> drivers/staging/media/tegra/video.h | 34 +
>>> 14 files changed, 3352 insertions(+)
>>> create mode 100644 drivers/staging/media/tegra/Kconfig
>>> create mode 100644 drivers/staging/media/tegra/Makefile
>>> create mode 100644 drivers/staging/media/tegra/TODO
>>> create mode 100644 drivers/staging/media/tegra/common.h
>>> create mode 100644 drivers/staging/media/tegra/csi.c
>>> create mode 100644 drivers/staging/media/tegra/csi.h
>>> create mode 100644 drivers/staging/media/tegra/tegra210.c
>>> create mode 100644 drivers/staging/media/tegra/tegra210.h
>>> create mode 100644 drivers/staging/media/tegra/vi.c
>>> create mode 100644 drivers/staging/media/tegra/vi.h
>>> create mode 100644 drivers/staging/media/tegra/video.c
>>> create mode 100644 drivers/staging/media/tegra/video.h
>> With 'make menuconfig' I get this:
>>
>> scripts/kconfig/mconf Kconfig
>>
>> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
>> Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM &&
>> COMPILE_TEST [=y])
>> Selected by [y]:
>> - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] &&
>> MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>>
>> This is an x86_64 build with COMPILE_TEST set. I can provide my full
>> .config if you need it.
>>
>> CONFIG_TEGRA_HOST1X=y
>> CONFIG_VIDEO_TEGRA=y
>>
>> Regards,
>>
>> Hans
>
> Hi Hans,
>
> In v7, changed Kconfig to remove ARM. But looks like we should limit
>
> TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on
> x86_64 shows above warning.
>
> We should limit compile to ARM for CONFIG_VIDEO_TEGRA.
>
> Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST
> like I had in previous version. Sorry about this.
>
>
> Also, I see some changes went into latest linux-next staging media
> Kconfig, So, will have my patches on top of today's linux-next.
VIDEO_TEGRA should depend on TEGRA_HOST1X and not select it.
depends on (ARCH_TEGRA && TEGRA_HOST1X) || COMPILE_TEST
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 22:55 ` Dmitry Osipenko
@ 2020-04-23 22:59 ` Sowjanya Komatineni
2020-04-23 23:03 ` Dmitry Osipenko
0 siblings, 1 reply; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-23 22:59 UTC (permalink / raw)
To: Dmitry Osipenko, Hans Verkuil, thierry.reding, jonathanh, frankc,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
On 4/23/20 3:55 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 23.04.2020 19:50, Sowjanya Komatineni пишет:
>> On 4/23/20 12:48 AM, Hans Verkuil wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>>>> Tegra210 contains a powerful Video Input (VI) hardware controller
>>>> which can support up to 6 MIPI CSI camera sensors.
>>>>
>>>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>>>> capture from an external camera sensor connected to CSI or from
>>>> built-in test pattern generator.
>>>>
>>>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>>>
>>>> This patch adds a v4l2 capture driver with media interface for
>>>> Tegra210 built-in CSI to VI test pattern generator.
>>>>
>>>> This patch includes TPG support only and all the video pipeline
>>>> configuration happens through the video device node.
>>>>
>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> drivers/staging/media/Kconfig | 2 +
>>>> drivers/staging/media/Makefile | 1 +
>>>> drivers/staging/media/tegra/Kconfig | 13 +
>>>> drivers/staging/media/tegra/Makefile | 8 +
>>>> drivers/staging/media/tegra/TODO | 10 +
>>>> drivers/staging/media/tegra/common.h | 262 ++++++++
>>>> drivers/staging/media/tegra/csi.c | 606 +++++++++++++++++
>>>> drivers/staging/media/tegra/csi.h | 149 +++++
>>>> drivers/staging/media/tegra/tegra210.c | 709 ++++++++++++++++++++
>>>> drivers/staging/media/tegra/tegra210.h | 190 ++++++
>>>> drivers/staging/media/tegra/vi.c | 1132
>>>> ++++++++++++++++++++++++++++++++
>>>> drivers/staging/media/tegra/vi.h | 83 +++
>>>> drivers/staging/media/tegra/video.c | 153 +++++
>>>> drivers/staging/media/tegra/video.h | 34 +
>>>> 14 files changed, 3352 insertions(+)
>>>> create mode 100644 drivers/staging/media/tegra/Kconfig
>>>> create mode 100644 drivers/staging/media/tegra/Makefile
>>>> create mode 100644 drivers/staging/media/tegra/TODO
>>>> create mode 100644 drivers/staging/media/tegra/common.h
>>>> create mode 100644 drivers/staging/media/tegra/csi.c
>>>> create mode 100644 drivers/staging/media/tegra/csi.h
>>>> create mode 100644 drivers/staging/media/tegra/tegra210.c
>>>> create mode 100644 drivers/staging/media/tegra/tegra210.h
>>>> create mode 100644 drivers/staging/media/tegra/vi.c
>>>> create mode 100644 drivers/staging/media/tegra/vi.h
>>>> create mode 100644 drivers/staging/media/tegra/video.c
>>>> create mode 100644 drivers/staging/media/tegra/video.h
>>> With 'make menuconfig' I get this:
>>>
>>> scripts/kconfig/mconf Kconfig
>>>
>>> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
>>> Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM &&
>>> COMPILE_TEST [=y])
>>> Selected by [y]:
>>> - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] &&
>>> MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>>>
>>> This is an x86_64 build with COMPILE_TEST set. I can provide my full
>>> .config if you need it.
>>>
>>> CONFIG_TEGRA_HOST1X=y
>>> CONFIG_VIDEO_TEGRA=y
>>>
>>> Regards,
>>>
>>> Hans
>> Hi Hans,
>>
>> In v7, changed Kconfig to remove ARM. But looks like we should limit
>>
>> TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on
>> x86_64 shows above warning.
>>
>> We should limit compile to ARM for CONFIG_VIDEO_TEGRA.
>>
>> Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST
>> like I had in previous version. Sorry about this.
>>
>>
>> Also, I see some changes went into latest linux-next staging media
>> Kconfig, So, will have my patches on top of today's linux-next.
> VIDEO_TEGRA should depend on TEGRA_HOST1X and not select it.
>
> depends on (ARCH_TEGRA && TEGRA_HOST1X) || COMPILE_TEST
Was selecting it to auto-select Tegra host1x when video_tegra is enabled.
Yes, can use depends on
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 22:59 ` Sowjanya Komatineni
@ 2020-04-23 23:03 ` Dmitry Osipenko
2020-04-23 23:14 ` Dmitry Osipenko
0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Osipenko @ 2020-04-23 23:03 UTC (permalink / raw)
To: Sowjanya Komatineni, Hans Verkuil, thierry.reding, jonathanh,
frankc, sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
24.04.2020 01:59, Sowjanya Komatineni пишет:
>
> On 4/23/20 3:55 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 23.04.2020 19:50, Sowjanya Komatineni пишет:
>>> On 4/23/20 12:48 AM, Hans Verkuil wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>>>>> Tegra210 contains a powerful Video Input (VI) hardware controller
>>>>> which can support up to 6 MIPI CSI camera sensors.
>>>>>
>>>>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>>>>> capture from an external camera sensor connected to CSI or from
>>>>> built-in test pattern generator.
>>>>>
>>>>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>>>>
>>>>> This patch adds a v4l2 capture driver with media interface for
>>>>> Tegra210 built-in CSI to VI test pattern generator.
>>>>>
>>>>> This patch includes TPG support only and all the video pipeline
>>>>> configuration happens through the video device node.
>>>>>
>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> drivers/staging/media/Kconfig | 2 +
>>>>> drivers/staging/media/Makefile | 1 +
>>>>> drivers/staging/media/tegra/Kconfig | 13 +
>>>>> drivers/staging/media/tegra/Makefile | 8 +
>>>>> drivers/staging/media/tegra/TODO | 10 +
>>>>> drivers/staging/media/tegra/common.h | 262 ++++++++
>>>>> drivers/staging/media/tegra/csi.c | 606 +++++++++++++++++
>>>>> drivers/staging/media/tegra/csi.h | 149 +++++
>>>>> drivers/staging/media/tegra/tegra210.c | 709 ++++++++++++++++++++
>>>>> drivers/staging/media/tegra/tegra210.h | 190 ++++++
>>>>> drivers/staging/media/tegra/vi.c | 1132
>>>>> ++++++++++++++++++++++++++++++++
>>>>> drivers/staging/media/tegra/vi.h | 83 +++
>>>>> drivers/staging/media/tegra/video.c | 153 +++++
>>>>> drivers/staging/media/tegra/video.h | 34 +
>>>>> 14 files changed, 3352 insertions(+)
>>>>> create mode 100644 drivers/staging/media/tegra/Kconfig
>>>>> create mode 100644 drivers/staging/media/tegra/Makefile
>>>>> create mode 100644 drivers/staging/media/tegra/TODO
>>>>> create mode 100644 drivers/staging/media/tegra/common.h
>>>>> create mode 100644 drivers/staging/media/tegra/csi.c
>>>>> create mode 100644 drivers/staging/media/tegra/csi.h
>>>>> create mode 100644 drivers/staging/media/tegra/tegra210.c
>>>>> create mode 100644 drivers/staging/media/tegra/tegra210.h
>>>>> create mode 100644 drivers/staging/media/tegra/vi.c
>>>>> create mode 100644 drivers/staging/media/tegra/vi.h
>>>>> create mode 100644 drivers/staging/media/tegra/video.c
>>>>> create mode 100644 drivers/staging/media/tegra/video.h
>>>> With 'make menuconfig' I get this:
>>>>
>>>> scripts/kconfig/mconf Kconfig
>>>>
>>>> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
>>>> Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM &&
>>>> COMPILE_TEST [=y])
>>>> Selected by [y]:
>>>> - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] &&
>>>> MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>>>>
>>>> This is an x86_64 build with COMPILE_TEST set. I can provide my full
>>>> .config if you need it.
>>>>
>>>> CONFIG_TEGRA_HOST1X=y
>>>> CONFIG_VIDEO_TEGRA=y
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>> Hi Hans,
>>>
>>> In v7, changed Kconfig to remove ARM. But looks like we should limit
>>>
>>> TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on
>>> x86_64 shows above warning.
>>>
>>> We should limit compile to ARM for CONFIG_VIDEO_TEGRA.
>>>
>>> Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST
>>> like I had in previous version. Sorry about this.
>>>
>>>
>>> Also, I see some changes went into latest linux-next staging media
>>> Kconfig, So, will have my patches on top of today's linux-next.
>> VIDEO_TEGRA should depend on TEGRA_HOST1X and not select it.
>>
>> depends on (ARCH_TEGRA && TEGRA_HOST1X) || COMPILE_TEST
>
> Was selecting it to auto-select Tegra host1x when video_tegra is enabled.
>
> Yes, can use depends on
>
BTW, perhaps ARCH_TEGRA and COMPILE_TEST aren't needed since
TEGRA_HOST1X already depends on them, so just:
depends on TEGRA_HOST1X
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 23:03 ` Dmitry Osipenko
@ 2020-04-23 23:14 ` Dmitry Osipenko
0 siblings, 0 replies; 29+ messages in thread
From: Dmitry Osipenko @ 2020-04-23 23:14 UTC (permalink / raw)
To: Sowjanya Komatineni, Hans Verkuil, thierry.reding, jonathanh,
frankc, sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
24.04.2020 02:03, Dmitry Osipenko пишет:
> 24.04.2020 01:59, Sowjanya Komatineni пишет:
>>
>> On 4/23/20 3:55 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 23.04.2020 19:50, Sowjanya Komatineni пишет:
>>>> On 4/23/20 12:48 AM, Hans Verkuil wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> On 22/04/2020 08:18, Sowjanya Komatineni wrote:
>>>>>> Tegra210 contains a powerful Video Input (VI) hardware controller
>>>>>> which can support up to 6 MIPI CSI camera sensors.
>>>>>>
>>>>>> Each Tegra CSI port can be one-to-one mapped to VI channel and can
>>>>>> capture from an external camera sensor connected to CSI or from
>>>>>> built-in test pattern generator.
>>>>>>
>>>>>> Tegra210 supports built-in test pattern generator from CSI to VI.
>>>>>>
>>>>>> This patch adds a v4l2 capture driver with media interface for
>>>>>> Tegra210 built-in CSI to VI test pattern generator.
>>>>>>
>>>>>> This patch includes TPG support only and all the video pipeline
>>>>>> configuration happens through the video device node.
>>>>>>
>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>> ---
>>>>>> drivers/staging/media/Kconfig | 2 +
>>>>>> drivers/staging/media/Makefile | 1 +
>>>>>> drivers/staging/media/tegra/Kconfig | 13 +
>>>>>> drivers/staging/media/tegra/Makefile | 8 +
>>>>>> drivers/staging/media/tegra/TODO | 10 +
>>>>>> drivers/staging/media/tegra/common.h | 262 ++++++++
>>>>>> drivers/staging/media/tegra/csi.c | 606 +++++++++++++++++
>>>>>> drivers/staging/media/tegra/csi.h | 149 +++++
>>>>>> drivers/staging/media/tegra/tegra210.c | 709 ++++++++++++++++++++
>>>>>> drivers/staging/media/tegra/tegra210.h | 190 ++++++
>>>>>> drivers/staging/media/tegra/vi.c | 1132
>>>>>> ++++++++++++++++++++++++++++++++
>>>>>> drivers/staging/media/tegra/vi.h | 83 +++
>>>>>> drivers/staging/media/tegra/video.c | 153 +++++
>>>>>> drivers/staging/media/tegra/video.h | 34 +
>>>>>> 14 files changed, 3352 insertions(+)
>>>>>> create mode 100644 drivers/staging/media/tegra/Kconfig
>>>>>> create mode 100644 drivers/staging/media/tegra/Makefile
>>>>>> create mode 100644 drivers/staging/media/tegra/TODO
>>>>>> create mode 100644 drivers/staging/media/tegra/common.h
>>>>>> create mode 100644 drivers/staging/media/tegra/csi.c
>>>>>> create mode 100644 drivers/staging/media/tegra/csi.h
>>>>>> create mode 100644 drivers/staging/media/tegra/tegra210.c
>>>>>> create mode 100644 drivers/staging/media/tegra/tegra210.h
>>>>>> create mode 100644 drivers/staging/media/tegra/vi.c
>>>>>> create mode 100644 drivers/staging/media/tegra/vi.h
>>>>>> create mode 100644 drivers/staging/media/tegra/video.c
>>>>>> create mode 100644 drivers/staging/media/tegra/video.h
>>>>> With 'make menuconfig' I get this:
>>>>>
>>>>> scripts/kconfig/mconf Kconfig
>>>>>
>>>>> WARNING: unmet direct dependencies detected for TEGRA_HOST1X
>>>>> Depends on [n]: HAS_IOMEM [=y] && (ARCH_TEGRA || ARM &&
>>>>> COMPILE_TEST [=y])
>>>>> Selected by [y]:
>>>>> - VIDEO_TEGRA [=y] && STAGING [=y] && STAGING_MEDIA [=y] &&
>>>>> MEDIA_SUPPORT [=y] && (ARCH_TEGRA || COMPILE_TEST [=y])
>>>>>
>>>>> This is an x86_64 build with COMPILE_TEST set. I can provide my full
>>>>> .config if you need it.
>>>>>
>>>>> CONFIG_TEGRA_HOST1X=y
>>>>> CONFIG_VIDEO_TEGRA=y
>>>>>
>>>>> Regards,
>>>>>
>>>>> Hans
>>>> Hi Hans,
>>>>
>>>> In v7, changed Kconfig to remove ARM. But looks like we should limit
>>>>
>>>> TEGRA_HOST1X also limits compile to ARM only so running VIDEO_TEGRA on
>>>> x86_64 shows above warning.
>>>>
>>>> We should limit compile to ARM for CONFIG_VIDEO_TEGRA.
>>>>
>>>> Will update CONFIG_VIDEO_TEGRA dependency to use ARM && COMPILE_TEST
>>>> like I had in previous version. Sorry about this.
>>>>
>>>>
>>>> Also, I see some changes went into latest linux-next staging media
>>>> Kconfig, So, will have my patches on top of today's linux-next.
>>> VIDEO_TEGRA should depend on TEGRA_HOST1X and not select it.
>>>
>>> depends on (ARCH_TEGRA && TEGRA_HOST1X) || COMPILE_TEST
>>
>> Was selecting it to auto-select Tegra host1x when video_tegra is enabled.
>>
>> Yes, can use depends on
>>
>
> BTW, perhaps ARCH_TEGRA and COMPILE_TEST aren't needed since
> TEGRA_HOST1X already depends on them, so just:
>
> depends on TEGRA_HOST1X
>
Although, I guess TEGRA_HOST1X isn't really needed for compile-testing,
and thus, it won't hurt to drop that dependency for testing:
depends on TEGRA_HOST1X || COMPILE_TEST
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
[not found] ` <1587536339-4030-7-git-send-email-skomatineni@nvidia.com>
2020-04-23 7:48 ` [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver Hans Verkuil
@ 2020-04-23 23:16 ` Dmitry Osipenko
2020-04-23 23:19 ` Sowjanya Komatineni
1 sibling, 1 reply; 29+ messages in thread
From: Dmitry Osipenko @ 2020-04-23 23:16 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
22.04.2020 09:18, Sowjanya Komatineni пишет:
> +static int chan_capture_kthread_start(void *data)
> +{
> + struct tegra_vi_channel *chan = data;
> + struct tegra_channel_buffer *buf;
> + int err = 0;
> +
> + set_freezable();
> +
> + while (1) {
> + try_to_freeze();
> +
> + wait_event_interruptible(chan->start_wait,
> + !list_empty(&chan->capture) ||
> + kthread_should_stop());
> +
> + if (kthread_should_stop())
> + break;
> +
> + /*
> + * Source is not streaming if error is non-zero.
> + * So, do not dequeue buffers on capture error.
> + */
> + if (err)
> + continue;
This will result in an endless loop, I suppose it wasn't the intention.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 23:16 ` Dmitry Osipenko
@ 2020-04-23 23:19 ` Sowjanya Komatineni
2020-04-23 23:20 ` Sowjanya Komatineni
0 siblings, 1 reply; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-23 23:19 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>> +static int chan_capture_kthread_start(void *data)
>> +{
>> + struct tegra_vi_channel *chan = data;
>> + struct tegra_channel_buffer *buf;
>> + int err = 0;
>> +
>> + set_freezable();
>> +
>> + while (1) {
>> + try_to_freeze();
>> +
>> + wait_event_interruptible(chan->start_wait,
>> + !list_empty(&chan->capture) ||
>> + kthread_should_stop());
>> +
>> + if (kthread_should_stop())
>> + break;
>> +
>> + /*
>> + * Source is not streaming if error is non-zero.
>> + * So, do not dequeue buffers on capture error.
>> + */
>> + if (err)
>> + continue;
> This will result in an endless loop, I suppose it wasn't the intention.
no it will not. on error we report vb2_queue_error which will do
streaming stop request.
So thread will be stopped on streaming stop request thru kthread stop signal
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 23:19 ` Sowjanya Komatineni
@ 2020-04-23 23:20 ` Sowjanya Komatineni
2020-04-23 23:25 ` Dmitry Osipenko
0 siblings, 1 reply; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-23 23:20 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>
> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>> +static int chan_capture_kthread_start(void *data)
>>> +{
>>> + struct tegra_vi_channel *chan = data;
>>> + struct tegra_channel_buffer *buf;
>>> + int err = 0;
>>> +
>>> + set_freezable();
>>> +
>>> + while (1) {
>>> + try_to_freeze();
>>> +
>>> + wait_event_interruptible(chan->start_wait,
>>> + !list_empty(&chan->capture) ||
>>> + kthread_should_stop());
>>> +
>>> + if (kthread_should_stop())
>>> + break;
>>> +
>>> + /*
>>> + * Source is not streaming if error is non-zero.
>>> + * So, do not dequeue buffers on capture error.
>>> + */
>>> + if (err)
>>> + continue;
>> This will result in an endless loop, I suppose it wasn't the intention.
>
> no it will not. on error we report vb2_queue_error which will do
> streaming stop request.
>
> So thread will be stopped on streaming stop request thru kthread stop
> signal
To be clear on error it reports vb2 queue error and waits for stop
streaming to happen
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 23:20 ` Sowjanya Komatineni
@ 2020-04-23 23:25 ` Dmitry Osipenko
2020-04-23 23:50 ` Sowjanya Komatineni
0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Osipenko @ 2020-04-23 23:25 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
24.04.2020 02:20, Sowjanya Komatineni пишет:
>
> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>
>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>> +static int chan_capture_kthread_start(void *data)
>>>> +{
>>>> + struct tegra_vi_channel *chan = data;
>>>> + struct tegra_channel_buffer *buf;
>>>> + int err = 0;
>>>> +
>>>> + set_freezable();
>>>> +
>>>> + while (1) {
>>>> + try_to_freeze();
>>>> +
>>>> + wait_event_interruptible(chan->start_wait,
>>>> + !list_empty(&chan->capture) ||
>>>> + kthread_should_stop());
>>>> +
>>>> + if (kthread_should_stop())
>>>> + break;
>>>> +
>>>> + /*
>>>> + * Source is not streaming if error is non-zero.
>>>> + * So, do not dequeue buffers on capture error.
>>>> + */
>>>> + if (err)
>>>> + continue;
>>> This will result in an endless loop, I suppose it wasn't the intention.
>>
>> no it will not. on error we report vb2_queue_error which will do
>> streaming stop request.
>>
>> So thread will be stopped on streaming stop request thru kthread stop
>> signal
> To be clear on error it reports vb2 queue error and waits for stop
> streaming to happen
If thread should exit on error, then it should do it on the actual
error. Otherwise it looks very error-prone.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 23:25 ` Dmitry Osipenko
@ 2020-04-23 23:50 ` Sowjanya Komatineni
2020-04-24 0:42 ` Dmitry Osipenko
0 siblings, 1 reply; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-23 23:50 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>> +static int chan_capture_kthread_start(void *data)
>>>>> +{
>>>>> + struct tegra_vi_channel *chan = data;
>>>>> + struct tegra_channel_buffer *buf;
>>>>> + int err = 0;
>>>>> +
>>>>> + set_freezable();
>>>>> +
>>>>> + while (1) {
>>>>> + try_to_freeze();
>>>>> +
>>>>> + wait_event_interruptible(chan->start_wait,
>>>>> + !list_empty(&chan->capture) ||
>>>>> + kthread_should_stop());
>>>>> +
>>>>> + if (kthread_should_stop())
>>>>> + break;
>>>>> +
>>>>> + /*
>>>>> + * Source is not streaming if error is non-zero.
>>>>> + * So, do not dequeue buffers on capture error.
>>>>> + */
>>>>> + if (err)
>>>>> + continue;
>>>> This will result in an endless loop, I suppose it wasn't the intention.
>>> no it will not. on error we report vb2_queue_error which will do
>>> streaming stop request.
>>>
>>> So thread will be stopped on streaming stop request thru kthread stop
>>> signal
>> To be clear on error it reports vb2 queue error and waits for stop
>> streaming to happen
> If thread should exit on error, then it should do it on the actual
> error. Otherwise it looks very error-prone.
When v4l2 drivers indicate fatal error through vb2_queue_error, queue
error flag is set and wakes up all processes waiting on queue along
with polling reporting EPOLLERR and also reporting error for queuing
and dequeuing buffers. Stream stop will surely happen which stops the
thread.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-23 23:50 ` Sowjanya Komatineni
@ 2020-04-24 0:42 ` Dmitry Osipenko
2020-04-24 0:51 ` Sowjanya Komatineni
0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Osipenko @ 2020-04-24 0:42 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
24.04.2020 02:50, Sowjanya Komatineni пишет:
>
> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>> +{
>>>>>> + struct tegra_vi_channel *chan = data;
>>>>>> + struct tegra_channel_buffer *buf;
>>>>>> + int err = 0;
>>>>>> +
>>>>>> + set_freezable();
>>>>>> +
>>>>>> + while (1) {
>>>>>> + try_to_freeze();
>>>>>> +
>>>>>> + wait_event_interruptible(chan->start_wait,
>>>>>> + !list_empty(&chan->capture) ||
>>>>>> + kthread_should_stop());
>>>>>> +
>>>>>> + if (kthread_should_stop())
>>>>>> + break;
>>>>>> +
>>>>>> + /*
>>>>>> + * Source is not streaming if error is non-zero.
>>>>>> + * So, do not dequeue buffers on capture error.
>>>>>> + */
>>>>>> + if (err)
>>>>>> + continue;
>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>> intention.
>>>> no it will not. on error we report vb2_queue_error which will do
>>>> streaming stop request.
>>>>
>>>> So thread will be stopped on streaming stop request thru kthread stop
>>>> signal
>>> To be clear on error it reports vb2 queue error and waits for stop
>>> streaming to happen
>> If thread should exit on error, then it should do it on the actual
>> error. Otherwise it looks very error-prone.
> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
> error flag is set and wakes up all processes waiting on queue along
> with polling reporting EPOLLERR and also reporting error for queuing
> and dequeuing buffers. Stream stop will surely happen which stops the
> thread.
This doesn't explain what is the point of continuing to loop instead of
exiting immediately on error.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-24 0:42 ` Dmitry Osipenko
@ 2020-04-24 0:51 ` Sowjanya Komatineni
2020-04-24 1:08 ` Sowjanya Komatineni
0 siblings, 1 reply; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-24 0:51 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 24.04.2020 02:50, Sowjanya Komatineni пишет:
>> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>>> +{
>>>>>>> + struct tegra_vi_channel *chan = data;
>>>>>>> + struct tegra_channel_buffer *buf;
>>>>>>> + int err = 0;
>>>>>>> +
>>>>>>> + set_freezable();
>>>>>>> +
>>>>>>> + while (1) {
>>>>>>> + try_to_freeze();
>>>>>>> +
>>>>>>> + wait_event_interruptible(chan->start_wait,
>>>>>>> + !list_empty(&chan->capture) ||
>>>>>>> + kthread_should_stop());
>>>>>>> +
>>>>>>> + if (kthread_should_stop())
>>>>>>> + break;
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * Source is not streaming if error is non-zero.
>>>>>>> + * So, do not dequeue buffers on capture error.
>>>>>>> + */
>>>>>>> + if (err)
>>>>>>> + continue;
>>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>>> intention.
>>>>> no it will not. on error we report vb2_queue_error which will do
>>>>> streaming stop request.
>>>>>
>>>>> So thread will be stopped on streaming stop request thru kthread stop
>>>>> signal
>>>> To be clear on error it reports vb2 queue error and waits for stop
>>>> streaming to happen
>>> If thread should exit on error, then it should do it on the actual
>>> error. Otherwise it looks very error-prone.
>> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
>> error flag is set and wakes up all processes waiting on queue along
>> with polling reporting EPOLLERR and also reporting error for queuing
>> and dequeuing buffers. Stream stop will surely happen which stops the
>> thread.
> This doesn't explain what is the point of continuing to loop instead of
> exiting immediately on error.
We are using 2 threads and when capture start error happens, we can stop
capture_start thread immediately but capture_finish thread will still
run for any outstanding buffers.
So, as it makes no diff stopping both threads during stream stop which
will definitely happen on error and when we don't dequeue buffers
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-24 0:51 ` Sowjanya Komatineni
@ 2020-04-24 1:08 ` Sowjanya Komatineni
2020-04-24 2:09 ` Dmitry Osipenko
0 siblings, 1 reply; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-24 1:08 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
On 4/23/20 5:51 PM, Sowjanya Komatineni wrote:
>
> On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 24.04.2020 02:50, Sowjanya Komatineni пишет:
>>> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>>>> External email: Use caution opening links or attachments
>>>>>>>
>>>>>>>
>>>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>>>> +{
>>>>>>>> + struct tegra_vi_channel *chan = data;
>>>>>>>> + struct tegra_channel_buffer *buf;
>>>>>>>> + int err = 0;
>>>>>>>> +
>>>>>>>> + set_freezable();
>>>>>>>> +
>>>>>>>> + while (1) {
>>>>>>>> + try_to_freeze();
>>>>>>>> +
>>>>>>>> + wait_event_interruptible(chan->start_wait,
>>>>>>>> + !list_empty(&chan->capture) ||
>>>>>>>> + kthread_should_stop());
>>>>>>>> +
>>>>>>>> + if (kthread_should_stop())
>>>>>>>> + break;
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * Source is not streaming if error is non-zero.
>>>>>>>> + * So, do not dequeue buffers on capture error.
>>>>>>>> + */
>>>>>>>> + if (err)
>>>>>>>> + continue;
>>>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>>>> intention.
>>>>>> no it will not. on error we report vb2_queue_error which will do
>>>>>> streaming stop request.
>>>>>>
>>>>>> So thread will be stopped on streaming stop request thru kthread
>>>>>> stop
>>>>>> signal
>>>>> To be clear on error it reports vb2 queue error and waits for stop
>>>>> streaming to happen
>>>> If thread should exit on error, then it should do it on the actual
>>>> error. Otherwise it looks very error-prone.
>>> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
>>> error flag is set and wakes up all processes waiting on queue along
>>> with polling reporting EPOLLERR and also reporting error for queuing
>>> and dequeuing buffers. Stream stop will surely happen which stops the
>>> thread.
>> This doesn't explain what is the point of continuing to loop instead of
>> exiting immediately on error.
>
> We are using 2 threads and when capture start error happens, we can
> stop capture_start thread immediately but capture_finish thread will
> still run for any outstanding buffers.
>
> So, as it makes no diff stopping both threads during stream stop which
> will definitely happen on error and when we don't dequeue buffers
>
Also there will be an issue if we break on error immediately during
stop_streaming -> kthread_stop()
As stop streaming can happen any time, we do kthread_stop and in case of
error if we stop thread and on stop streaming kthread_stop might crash
as kthread_stop can only be called on running thread
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-24 1:08 ` Sowjanya Komatineni
@ 2020-04-24 2:09 ` Dmitry Osipenko
2020-04-24 2:12 ` Sowjanya Komatineni
0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Osipenko @ 2020-04-24 2:09 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
24.04.2020 04:08, Sowjanya Komatineni пишет:
>
> On 4/23/20 5:51 PM, Sowjanya Komatineni wrote:
>>
>> On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 24.04.2020 02:50, Sowjanya Komatineni пишет:
>>>> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>>>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>>
>>>>>>>>
>>>>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>>>>> +{
>>>>>>>>> + struct tegra_vi_channel *chan = data;
>>>>>>>>> + struct tegra_channel_buffer *buf;
>>>>>>>>> + int err = 0;
>>>>>>>>> +
>>>>>>>>> + set_freezable();
>>>>>>>>> +
>>>>>>>>> + while (1) {
>>>>>>>>> + try_to_freeze();
>>>>>>>>> +
>>>>>>>>> + wait_event_interruptible(chan->start_wait,
>>>>>>>>> + !list_empty(&chan->capture) ||
>>>>>>>>> + kthread_should_stop());
>>>>>>>>> +
>>>>>>>>> + if (kthread_should_stop())
>>>>>>>>> + break;
>>>>>>>>> +
>>>>>>>>> + /*
>>>>>>>>> + * Source is not streaming if error is non-zero.
>>>>>>>>> + * So, do not dequeue buffers on capture error.
>>>>>>>>> + */
>>>>>>>>> + if (err)
>>>>>>>>> + continue;
>>>>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>>>>> intention.
>>>>>>> no it will not. on error we report vb2_queue_error which will do
>>>>>>> streaming stop request.
>>>>>>>
>>>>>>> So thread will be stopped on streaming stop request thru kthread
>>>>>>> stop
>>>>>>> signal
>>>>>> To be clear on error it reports vb2 queue error and waits for stop
>>>>>> streaming to happen
>>>>> If thread should exit on error, then it should do it on the actual
>>>>> error. Otherwise it looks very error-prone.
>>>> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
>>>> error flag is set and wakes up all processes waiting on queue along
>>>> with polling reporting EPOLLERR and also reporting error for queuing
>>>> and dequeuing buffers. Stream stop will surely happen which stops the
>>>> thread.
>>> This doesn't explain what is the point of continuing to loop instead of
>>> exiting immediately on error.
>>
>> We are using 2 threads and when capture start error happens, we can
>> stop capture_start thread immediately but capture_finish thread will
>> still run for any outstanding buffers.
>>
>> So, as it makes no diff stopping both threads during stream stop which
>> will definitely happen on error and when we don't dequeue buffers
>>
> Also there will be an issue if we break on error immediately during
> stop_streaming -> kthread_stop()
>
> As stop streaming can happen any time, we do kthread_stop and in case of
> error if we stop thread and on stop streaming kthread_stop might crash
> as kthread_stop can only be called on running thread
>
This a better explanation, but still it's not good that there could be a
busy loop within the thread.
Should be better to sleep if error is set.
wait_event_interruptible(chan->start_wait,
(!err && !list_empty(&chan->capture)) ||
kthread_should_stop());
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver
2020-04-24 2:09 ` Dmitry Osipenko
@ 2020-04-24 2:12 ` Sowjanya Komatineni
0 siblings, 0 replies; 29+ messages in thread
From: Sowjanya Komatineni @ 2020-04-24 2:12 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, frankc, hverkuil,
sakari.ailus, helen.koike
Cc: sboyd, linux-media, devicetree, linux-clk, linux-tegra,
linux-kernel
On 4/23/20 7:09 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 24.04.2020 04:08, Sowjanya Komatineni пишет:
>> On 4/23/20 5:51 PM, Sowjanya Komatineni wrote:
>>> On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> 24.04.2020 02:50, Sowjanya Komatineni пишет:
>>>>> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>>>>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>>>>>> +{
>>>>>>>>>> + struct tegra_vi_channel *chan = data;
>>>>>>>>>> + struct tegra_channel_buffer *buf;
>>>>>>>>>> + int err = 0;
>>>>>>>>>> +
>>>>>>>>>> + set_freezable();
>>>>>>>>>> +
>>>>>>>>>> + while (1) {
>>>>>>>>>> + try_to_freeze();
>>>>>>>>>> +
>>>>>>>>>> + wait_event_interruptible(chan->start_wait,
>>>>>>>>>> + !list_empty(&chan->capture) ||
>>>>>>>>>> + kthread_should_stop());
>>>>>>>>>> +
>>>>>>>>>> + if (kthread_should_stop())
>>>>>>>>>> + break;
>>>>>>>>>> +
>>>>>>>>>> + /*
>>>>>>>>>> + * Source is not streaming if error is non-zero.
>>>>>>>>>> + * So, do not dequeue buffers on capture error.
>>>>>>>>>> + */
>>>>>>>>>> + if (err)
>>>>>>>>>> + continue;
>>>>>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>>>>>> intention.
>>>>>>>> no it will not. on error we report vb2_queue_error which will do
>>>>>>>> streaming stop request.
>>>>>>>>
>>>>>>>> So thread will be stopped on streaming stop request thru kthread
>>>>>>>> stop
>>>>>>>> signal
>>>>>>> To be clear on error it reports vb2 queue error and waits for stop
>>>>>>> streaming to happen
>>>>>> If thread should exit on error, then it should do it on the actual
>>>>>> error. Otherwise it looks very error-prone.
>>>>> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
>>>>> error flag is set and wakes up all processes waiting on queue along
>>>>> with polling reporting EPOLLERR and also reporting error for queuing
>>>>> and dequeuing buffers. Stream stop will surely happen which stops the
>>>>> thread.
>>>> This doesn't explain what is the point of continuing to loop instead of
>>>> exiting immediately on error.
>>> We are using 2 threads and when capture start error happens, we can
>>> stop capture_start thread immediately but capture_finish thread will
>>> still run for any outstanding buffers.
>>>
>>> So, as it makes no diff stopping both threads during stream stop which
>>> will definitely happen on error and when we don't dequeue buffers
>>>
>> Also there will be an issue if we break on error immediately during
>> stop_streaming -> kthread_stop()
>>
>> As stop streaming can happen any time, we do kthread_stop and in case of
>> error if we stop thread and on stop streaming kthread_stop might crash
>> as kthread_stop can only be called on running thread
>>
> This a better explanation, but still it's not good that there could be a
> busy loop within the thread.
>
> Should be better to sleep if error is set.
>
> wait_event_interruptible(chan->start_wait,
> (!err && !list_empty(&chan->capture)) ||
> kthread_should_stop());
ok, will add err to wait condition to let it sleep
^ permalink raw reply [flat|nested] 29+ messages in thread