Messages in this thread Patch in this message | | | Subject | Re: [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings | From | Sowjanya Komatineni <> | Date | Wed, 22 Apr 2020 10:57:39 -0700 |
| |
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
| |