lkml.org 
[lkml]   [2020]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings
From
Date

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

\
 
 \ /
  Last update: 2020-04-22 19:58    [W:2.250 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site