| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 22 Apr 2020 10:57:39 -0700
From: Sowjanya Komatineni <skomatineni@...dia.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
CC: <thierry.reding@...il.com>, <jonathanh@...dia.com>,
<frankc@...dia.com>, <hverkuil@...all.nl>, <sakari.ailus@....fi>,
<helen.koike@...labora.com>, <digetx@...il.com>,
<sboyd@...nel.org>, <linux-media@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-clk@...r.kernel.org>,
<linux-tegra@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v9 5/9] dt-binding: tegra: Add VI and CSI bindings
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@...dia.com>
>>>> Reviewed-by: Rob Herring <robh@...nel.org>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@...dia.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@...80000 {
>>>> + 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
Powered by blists - more mailing lists