diff mbox series

[RFC,v9,5/9] dt-binding: tegra: Add VI and CSI bindings

Message ID 1587536339-4030-6-git-send-email-skomatineni@nvidia.com
State Not Applicable, archived
Headers show
Series Add Tegra driver for video capture | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Sowjanya Komatineni April 22, 2020, 6:18 a.m. UTC
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(-)

Comments

Laurent Pinchart April 22, 2020, 5:20 p.m. UTC | #1
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 {
Sowjanya Komatineni April 22, 2020, 5:26 p.m. UTC | #2
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
Laurent Pinchart April 22, 2020, 5:47 p.m. UTC | #3
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 {
Sowjanya Komatineni April 22, 2020, 5:57 p.m. UTC | #4
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
diff mbox series

Patch

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 {