[v2,9/9] ARM: dts: chameleonv3: Add video device nodes

Message ID 20240221160215.484151-10-panikiel@google.com
State New
Headers
Series Add Chameleon v3 video support |

Commit Message

Paweł Anikiel Feb. 21, 2024, 4:02 p.m. UTC
  Add device nodes for the video system present on the Chameleon v3.
It consists of six framebuffers and two Intel Displayport receivers.

Signed-off-by: Paweł Anikiel <panikiel@google.com>
---
 .../socfpga/socfpga_arria10_chameleonv3.dts   | 152 ++++++++++++++++++
 1 file changed, 152 insertions(+)
  

Comments

Krzysztof Kozlowski Feb. 26, 2024, 9:15 a.m. UTC | #1
On 21/02/2024 17:02, Paweł Anikiel wrote:
> Add device nodes for the video system present on the Chameleon v3.
> It consists of six framebuffers and two Intel Displayport receivers.
> 
> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> ---

..

> +		dprx_sst: dp-receiver@c0064000 {
> +			compatible = "intel,dprx-20.0.1";
> +			reg = <0xc0064000 0x800>;
> +			interrupt-parent = <&dprx_sst_irq>;
> +			interrupts = <0 IRQ_TYPE_EDGE_RISING>;
> +			intel,max-link-rate = <0x1e>;

Rate is not in hex! Rate is in Hz, at least usually...

Fix your bindings...

Best regards,
Krzysztof
  
Paweł Anikiel Feb. 26, 2024, 11:09 a.m. UTC | #2
On Mon, Feb 26, 2024 at 10:15 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/02/2024 17:02, Paweł Anikiel wrote:
> > Add device nodes for the video system present on the Chameleon v3.
> > It consists of six framebuffers and two Intel Displayport receivers.
> >
> > Signed-off-by: Paweł Anikiel <panikiel@google.com>
> > ---
>
> ...
>
> > +             dprx_sst: dp-receiver@c0064000 {
> > +                     compatible = "intel,dprx-20.0.1";
> > +                     reg = <0xc0064000 0x800>;
> > +                     interrupt-parent = <&dprx_sst_irq>;
> > +                     interrupts = <0 IRQ_TYPE_EDGE_RISING>;
> > +                     intel,max-link-rate = <0x1e>;
>
> Rate is not in hex! Rate is in Hz, at least usually...
>
> Fix your bindings...

This is the DisplayPort link rate, for which the allowed values are
8.1 Gbps, 5.4 Gbps, 2.7 Gbps, or 1.62 Gbps. The standard way to encode
them (used in the DisplayPort DPCD registers and this device's
configuration) is by multiples of 0.27Gbps. This value (AFAIK) is
usually represented in hex, so 8.1Gbps would be 0x1e.
  
Krzysztof Kozlowski Feb. 26, 2024, 12:07 p.m. UTC | #3
On 26/02/2024 12:09, Paweł Anikiel wrote:
> On Mon, Feb 26, 2024 at 10:15 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 21/02/2024 17:02, Paweł Anikiel wrote:
>>> Add device nodes for the video system present on the Chameleon v3.
>>> It consists of six framebuffers and two Intel Displayport receivers.
>>>
>>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
>>> ---
>>
>> ...
>>
>>> +             dprx_sst: dp-receiver@c0064000 {
>>> +                     compatible = "intel,dprx-20.0.1";
>>> +                     reg = <0xc0064000 0x800>;
>>> +                     interrupt-parent = <&dprx_sst_irq>;
>>> +                     interrupts = <0 IRQ_TYPE_EDGE_RISING>;
>>> +                     intel,max-link-rate = <0x1e>;
>>
>> Rate is not in hex! Rate is in Hz, at least usually...
>>
>> Fix your bindings...
> 
> This is the DisplayPort link rate, for which the allowed values are
> 8.1 Gbps, 5.4 Gbps, 2.7 Gbps, or 1.62 Gbps. The standard way to encode
> them (used in the DisplayPort DPCD registers and this device's

Then it is in bps or some other units:

https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

> configuration) is by multiples of 0.27Gbps. This value (AFAIK) is
> usually represented in hex, so 8.1Gbps would be 0x1e.

No, the value is represented in logical units. Frequency in Hz. Rate in
bps/kbps/etc. Voltage in volts.

Best regards,
Krzysztof
  
Paweł Anikiel Feb. 26, 2024, 12:27 p.m. UTC | #4
On Mon, Feb 26, 2024 at 1:07 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/02/2024 12:09, Paweł Anikiel wrote:
> > On Mon, Feb 26, 2024 at 10:15 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 21/02/2024 17:02, Paweł Anikiel wrote:
> >>> Add device nodes for the video system present on the Chameleon v3.
> >>> It consists of six framebuffers and two Intel Displayport receivers.
> >>>
> >>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> >>> ---
> >>
> >> ...
> >>
> >>> +             dprx_sst: dp-receiver@c0064000 {
> >>> +                     compatible = "intel,dprx-20.0.1";
> >>> +                     reg = <0xc0064000 0x800>;
> >>> +                     interrupt-parent = <&dprx_sst_irq>;
> >>> +                     interrupts = <0 IRQ_TYPE_EDGE_RISING>;
> >>> +                     intel,max-link-rate = <0x1e>;
> >>
> >> Rate is not in hex! Rate is in Hz, at least usually...
> >>
> >> Fix your bindings...
> >
> > This is the DisplayPort link rate, for which the allowed values are
> > 8.1 Gbps, 5.4 Gbps, 2.7 Gbps, or 1.62 Gbps. The standard way to encode
> > them (used in the DisplayPort DPCD registers and this device's
>
> Then it is in bps or some other units:
>
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
>
> > configuration) is by multiples of 0.27Gbps. This value (AFAIK) is
> > usually represented in hex, so 8.1Gbps would be 0x1e.
>
> No, the value is represented in logical units. Frequency in Hz. Rate in
> bps/kbps/etc. Voltage in volts.

Okay, thanks for the info. So if I understand correctly, the max link
rate should be represented in bps in the devicetree, and then be
converted to the per 0.27Gbps value by the driver?

One problem is that the values here are too large to be represented in
bps (since the datatype is uint32). Can the property be in Mbps
instead?
  
Krzysztof Kozlowski Feb. 26, 2024, 5:30 p.m. UTC | #5
On 26/02/2024 13:27, Paweł Anikiel wrote:
> On Mon, Feb 26, 2024 at 1:07 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 26/02/2024 12:09, Paweł Anikiel wrote:
>>> On Mon, Feb 26, 2024 at 10:15 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 21/02/2024 17:02, Paweł Anikiel wrote:
>>>>> Add device nodes for the video system present on the Chameleon v3.
>>>>> It consists of six framebuffers and two Intel Displayport receivers.
>>>>>
>>>>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
>>>>> ---
>>>>
>>>> ...
>>>>
>>>>> +             dprx_sst: dp-receiver@c0064000 {
>>>>> +                     compatible = "intel,dprx-20.0.1";
>>>>> +                     reg = <0xc0064000 0x800>;
>>>>> +                     interrupt-parent = <&dprx_sst_irq>;
>>>>> +                     interrupts = <0 IRQ_TYPE_EDGE_RISING>;
>>>>> +                     intel,max-link-rate = <0x1e>;
>>>>
>>>> Rate is not in hex! Rate is in Hz, at least usually...
>>>>
>>>> Fix your bindings...
>>>
>>> This is the DisplayPort link rate, for which the allowed values are
>>> 8.1 Gbps, 5.4 Gbps, 2.7 Gbps, or 1.62 Gbps. The standard way to encode
>>> them (used in the DisplayPort DPCD registers and this device's
>>
>> Then it is in bps or some other units:
>>
>> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
>>
>>> configuration) is by multiples of 0.27Gbps. This value (AFAIK) is
>>> usually represented in hex, so 8.1Gbps would be 0x1e.
>>
>> No, the value is represented in logical units. Frequency in Hz. Rate in
>> bps/kbps/etc. Voltage in volts.
> 
> Okay, thanks for the info. So if I understand correctly, the max link
> rate should be represented in bps in the devicetree, and then be

or kbps

> converted to the per 0.27Gbps value by the driver?

If driver needs some register-based value, then yes.

> 
> One problem is that the values here are too large to be represented in
> bps (since the datatype is uint32). Can the property be in Mbps
> instead?

Can be. You can submit a patch to dtschema (patch to DT spec list or
github pull request) adding '-mbps' as well.

Best regards,
Krzysztof
  
Paweł Anikiel Feb. 27, 2024, 11:26 a.m. UTC | #6
On Mon, Feb 26, 2024 at 6:30 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/02/2024 13:27, Paweł Anikiel wrote:
> > On Mon, Feb 26, 2024 at 1:07 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 26/02/2024 12:09, Paweł Anikiel wrote:
> >>> On Mon, Feb 26, 2024 at 10:15 AM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> On 21/02/2024 17:02, Paweł Anikiel wrote:
> >>>>> Add device nodes for the video system present on the Chameleon v3.
> >>>>> It consists of six framebuffers and two Intel Displayport receivers.
> >>>>>
> >>>>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> >>>>> ---
> >>>>
> >>>> ...
> >>>>
> >>>>> +             dprx_sst: dp-receiver@c0064000 {
> >>>>> +                     compatible = "intel,dprx-20.0.1";
> >>>>> +                     reg = <0xc0064000 0x800>;
> >>>>> +                     interrupt-parent = <&dprx_sst_irq>;
> >>>>> +                     interrupts = <0 IRQ_TYPE_EDGE_RISING>;
> >>>>> +                     intel,max-link-rate = <0x1e>;
> >>>>
> >>>> Rate is not in hex! Rate is in Hz, at least usually...
> >>>>
> >>>> Fix your bindings...
> >>>
> >>> This is the DisplayPort link rate, for which the allowed values are
> >>> 8.1 Gbps, 5.4 Gbps, 2.7 Gbps, or 1.62 Gbps. The standard way to encode
> >>> them (used in the DisplayPort DPCD registers and this device's
> >>
> >> Then it is in bps or some other units:
> >>
> >> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> >>
> >>> configuration) is by multiples of 0.27Gbps. This value (AFAIK) is
> >>> usually represented in hex, so 8.1Gbps would be 0x1e.
> >>
> >> No, the value is represented in logical units. Frequency in Hz. Rate in
> >> bps/kbps/etc. Voltage in volts.
> >
> > Okay, thanks for the info. So if I understand correctly, the max link
> > rate should be represented in bps in the devicetree, and then be
>
> or kbps

The one that's already present in dtschema is kBps (kilobytes per
second) which isn't right for this case IMO.

>
> > converted to the per 0.27Gbps value by the driver?
>
> If driver needs some register-based value, then yes.
>
> >
> > One problem is that the values here are too large to be represented in
> > bps (since the datatype is uint32). Can the property be in Mbps
> > instead?
>
> Can be. You can submit a patch to dtschema (patch to DT spec list or
> github pull request) adding '-mbps' as well.

I sent a PR with both kbps and mbps.
  

Patch

diff --git a/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts b/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts
index 422d00cd4c74..2f48f30cb538 100644
--- a/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts
+++ b/arch/arm/boot/dts/intel/socfpga/socfpga_arria10_chameleonv3.dts
@@ -10,6 +10,158 @@  / {
 	compatible = "google,chameleon-v3", "enclustra,mercury-aa1",
 		     "altr,socfpga-arria10", "altr,socfpga";
 
+	soc {
+		fb0: video@c0060500 {
+			compatible = "google,chv3-fb";
+			reg = <0xc0060500 0x100>,
+			      <0xc0060f20 0x10>;
+			interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+			google,legacy-format;
+		};
+
+		fb_mst0: video@c0060600 {
+			compatible = "google,chv3-fb";
+			reg = <0xc0060600 0x100>,
+			      <0xc0060f30 0x10>;
+			interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
+
+			port {
+				fb_mst0_0: endpoint {
+					remote-endpoint = <&dprx_mst_0>;
+				};
+			};
+		};
+
+		fb_mst1: video@c0060700 {
+			compatible = "google,chv3-fb";
+			reg = <0xc0060700 0x100>,
+			      <0xc0060f40 0x10>;
+			interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
+
+			port {
+				fb_mst1_0: endpoint {
+					remote-endpoint = <&dprx_mst_1>;
+				};
+			};
+		};
+
+		fb_mst2: video@c0060800 {
+			compatible = "google,chv3-fb";
+			reg = <0xc0060800 0x100>,
+			      <0xc0060f50 0x10>;
+			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+
+			port {
+				fb_mst2_0: endpoint {
+					remote-endpoint = <&dprx_mst_2>;
+				};
+			};
+		};
+
+		fb_mst3: video@c0060900 {
+			compatible = "google,chv3-fb";
+			reg = <0xc0060900 0x100>,
+			      <0xc0060f60 0x10>;
+			interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+
+			port {
+				fb_mst3_0: endpoint {
+					remote-endpoint = <&dprx_mst_3>;
+				};
+			};
+		};
+
+		fb_sst: video@c0060a00 {
+			compatible = "google,chv3-fb";
+			reg = <0xc0060a00 0x100>,
+			      <0xc0060f70 0x10>;
+			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+
+			port {
+				fb_sst_0: endpoint {
+					remote-endpoint = <&dprx_sst_0>;
+				};
+			};
+		};
+
+		dprx_mst_irq: intc@c0060f80 {
+			compatible = "altr,pio-1.0";
+			reg = <0xc0060f80 0x10>;
+			interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
+			altr,interrupt-type = <IRQ_TYPE_EDGE_RISING>;
+			#interrupt-cells = <2>;
+			interrupt-controller;
+		};
+
+		dprx_sst_irq: intc@c0060fe0 {
+			compatible = "altr,pio-1.0";
+			reg = <0xc0060fe0 0x10>;
+			interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+			altr,interrupt-type = <IRQ_TYPE_EDGE_RISING>;
+			#interrupt-cells = <2>;
+			interrupt-controller;
+		};
+
+		dprx_mst: dp-receiver@c0062000 {
+			compatible = "intel,dprx-20.0.1";
+			reg = <0xc0062000 0x800>;
+			interrupt-parent = <&dprx_mst_irq>;
+			interrupts = <0 IRQ_TYPE_EDGE_RISING>;
+			intel,max-link-rate = <0x1e>;
+			intel,max-lane-count = <4>;
+			intel,multi-stream-support;
+			intel,max-stream-count = <4>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					dprx_mst_0: endpoint {
+						remote-endpoint = <&fb_mst0_0>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					dprx_mst_1: endpoint {
+						remote-endpoint = <&fb_mst1_0>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+					dprx_mst_2: endpoint {
+						remote-endpoint = <&fb_mst2_0>;
+					};
+				};
+
+				port@3 {
+					reg = <3>;
+					dprx_mst_3: endpoint {
+						remote-endpoint = <&fb_mst3_0>;
+					};
+				};
+			};
+		};
+
+		dprx_sst: dp-receiver@c0064000 {
+			compatible = "intel,dprx-20.0.1";
+			reg = <0xc0064000 0x800>;
+			interrupt-parent = <&dprx_sst_irq>;
+			interrupts = <0 IRQ_TYPE_EDGE_RISING>;
+			intel,max-link-rate = <0x1e>;
+			intel,max-lane-count = <4>;
+
+			port {
+				dprx_sst_0: endpoint {
+					remote-endpoint = <&fb_sst_0>;
+				};
+			};
+		};
+	};
+
 	aliases {
 		serial0 = &uart0;
 		i2c0 = &i2c0;