Hi Krzysztof,
On 7/21/24 21:09, Krzysztof Kozlowski wrote:
> On 16/07/2024 10:42, Aradhya Bhatia wrote:
>> The DSS in AM625 SoC has 2 OLDI TXes. Refer the OLDI schema to add the
>> support for the OLDI TXes.
>>
>> The AM625 DSS VP1 (port@0) can connect and control 2 OLDI TXes, to use
>> them in dual-link or cloned single-link OLDI modes. Add support for an
>> additional endpoint under the port@0 to accurately depict the data flow
>> path.
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@xxxxxx>
>> ---
>> .../bindings/display/ti/ti,am65x-dss.yaml | 135 ++++++++++++++++++
>> 1 file changed, 135 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> index 399d68986326..249597455d34 100644
>> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> @@ -91,6 +91,24 @@ properties:
>> For AM625 DSS, the internal DPI output port node from video
>> port 1.
>> For AM62A7 DSS, the port is tied off inside the SoC.
>> + properties:
>> + endpoint@0:
>> + $ref: /schemas/graph.yaml#/properties/endpoint
>> + description:
>> + For AM625 DSS, VP Connection to OLDI0.
>> + For AM65X DSS, OLDI output from the SoC.
>> +
>> + endpoint@1:
>> + $ref: /schemas/graph.yaml#/properties/endpoint
>> + description:
>> + For AM625 DSS, VP Connection to OLDI1.
>
> Eh, that's confusing. Why do you have graph to your children? Isn't this
> entirely pointless?
I am not sure I fully understand. The same display source video port can
connect up to 2 OLDI TXes - hence 2 endpoints which connect to the OLDI
that were described in the previous patch. The idea has been to
accurately depict the connections of the hardware.
What am I missing here?
side-note: I do realize, as I write this, that it has been quite a while
since you reviewed, and that you may have, rightfully, lost context.
I apologize for that.
>
>> +
>> + anyOf:
>> + - required:
>> + - endpoint
>> + - required:
>> + - endpoint@0
>> + - endpoint@1
>>
>> port@1:
>> $ref: /schemas/graph.yaml#/properties/port
>> @@ -112,6 +130,23 @@ properties:
>> Input memory (from main memory to dispc) bandwidth limit in
>> bytes per second
>>
>> + oldi-txes:
>> + type: object
>> + additionalProperties: true
>
> Why? This looks wrong.
This, I will admit, was a shot in the dark. The binding check asked me
that I was missing either this or unevaluatedProperties. I tried to make
sense of the two, but with little luck. Eventually, I went with this.
I could change it to unevaluatedProperties if that is indeed correct. I
could also use some comprehensive resource to understand this, if you
have something to recommend. =)
>
>> + properties:
>> + "#address-cells":
>> + const: 1
>> +
>> + "#size-cells":
>> + const: 0
>> +
>> + patternProperties:
>> + '^oldi_tx@[0-1]$':
>
> Please follow DTS coding style for naming.
Okay!
>
>> + type: object
>> + $ref: ti,am625-oldi.yaml#
>> + unevaluatedProperties: false
>> + description: OLDI transmitters connected to the DSS VPs
>> +
>> allOf:
>> - if:
>> properties:
>> @@ -123,6 +158,19 @@ allOf:
>> ports:
>> properties:
>> port@0: false
>> + oldi_txes: false
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: ti,am65x-dss
>> + then:
>> + properties:
>> + oldi_txes: false
>> + port@0:
>> + properties:
>> + endpoint@1: false
>>
>> required:
>> - compatible
>> @@ -171,3 +219,90 @@ examples:
>> };
>> };
>> };
>> +
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> + #include <dt-bindings/soc/ti,sci_pm_domain.h>
>> +
>> + bus {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + dss1: dss@30200000 {
>> + compatible = "ti,am625-dss";
>> + reg = <0x00 0x30200000 0x00 0x1000>, /* common */
>> + <0x00 0x30202000 0x00 0x1000>, /* vidl1 */
>> + <0x00 0x30206000 0x00 0x1000>, /* vid */
>> + <0x00 0x30207000 0x00 0x1000>, /* ovr1 */
>> + <0x00 0x30208000 0x00 0x1000>, /* ovr2 */
>> + <0x00 0x3020a000 0x00 0x1000>, /* vp1 */
>> + <0x00 0x3020b000 0x00 0x1000>, /* vp2 */
>> + <0x00 0x30201000 0x00 0x1000>; /* common1 */
>> + reg-names = "common", "vidl1", "vid",
>> + "ovr1", "ovr2", "vp1", "vp2", "common1";
>> + power-domains = <&k3_pds 186 TI_SCI_PD_EXCLUSIVE>;
>> + clocks = <&k3_clks 186 6>,
>> + <&vp1_clock>,
>> + <&k3_clks 186 2>;
>> + clock-names = "fck", "vp1", "vp2";
>> + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
>> + oldi-txes {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + oldi0: oldi@0 {
>
> You are duplicating the example from previous schema. No need. Keep
> only, one complete example.
Sure!
Regards
Aradhya
