diff mbox series

[2/2] dt-bindings: display: bridge: sil, sii9022: Add data-lines

Message ID 20241003082006.2728617-2-w.egorov@phytec.de (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/bridge: sii902x: Provide data-lines property to input endpoint | expand

Commit Message

Wadim Egorov Oct. 3, 2024, 8:20 a.m. UTC
The SI9022 HDMI transmitter can be configured with 16, 18, or 24 input
data lines. This commit introduces the data-lines property to the input
endpoint, specifying the number of parallel RGB input pins connected
to the transmitter.

Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
---
 .../bindings/display/bridge/sil,sii9022.yaml        | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski Oct. 3, 2024, 10:03 a.m. UTC | #1
On 03/10/2024 10:20, Wadim Egorov wrote:
> The SI9022 HDMI transmitter can be configured with 16, 18, or 24 input
> data lines. This commit introduces the data-lines property to the input

lines? lanes? What are lines? like pins?

> endpoint, specifying the number of parallel RGB input pins connected
> to the transmitter.
> 
> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> ---
>  .../bindings/display/bridge/sil,sii9022.yaml        | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
> index 5a69547ad3d7..24306f8eb107 100644
> --- a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
> @@ -81,9 +81,20 @@ properties:
>  
>      properties:
>        port@0:
> -        $ref: /schemas/graph.yaml#/properties/port
> +        unevaluatedProperties: false
> +        $ref: /schemas/graph.yaml#/$defs/port-base
>          description: Parallel RGB input port
>  
> +        properties:
> +          endpoint:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-lines:

No, this will confuse everyone. Considering lack of description how
anyone would figure out what this means?

I don't know much about RGB parallel port to advice how this should be
called. Can you describe the hardware more?

> +                $ref: /schemas/types.yaml#/definitions/uint32
> +                enum: [ 16, 18, 24 ]
> +
>        port@1:
>          $ref: /schemas/graph.yaml#/properties/port
>          description: HDMI output port

Best regards,
Krzysztof
Wadim Egorov Oct. 3, 2024, 11:56 a.m. UTC | #2
Am 03.10.24 um 12:03 schrieb Krzysztof Kozlowski:
> On 03/10/2024 10:20, Wadim Egorov wrote:
>> The SI9022 HDMI transmitter can be configured with 16, 18, or 24 input
>> data lines. This commit introduces the data-lines property to the input
> 
> lines? lanes? What are lines? like pins?

Yes, "lines" in this context refers to the number of pins used for the 
input pixel data bus, which can support 16, 18, or 24-bit wide data 
buses. These are parallel data lines (or pins) that carry uncompressed 
digital video to the HDMI transmitter.

> 
>> endpoint, specifying the number of parallel RGB input pins connected
>> to the transmitter.
>>
>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
>> ---
>>   .../bindings/display/bridge/sil,sii9022.yaml        | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
>> index 5a69547ad3d7..24306f8eb107 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
>> +++ b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
>> @@ -81,9 +81,20 @@ properties:
>>   
>>       properties:
>>         port@0:
>> -        $ref: /schemas/graph.yaml#/properties/port
>> +        unevaluatedProperties: false
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>>           description: Parallel RGB input port
>>   
>> +        properties:
>> +          endpoint:
>> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
>> +            unevaluatedProperties: false
>> +
>> +            properties:
>> +              data-lines:
> 
> No, this will confuse everyone. Considering lack of description how
> anyone would figure out what this means?

I guess from working with the hardware/reference manual and using this chip?

I don't think it is overly confusing, especially since the port is 
already described as the "Parallel RGB input port" which clearly implies 
the use of pins for data transmission.

I am open to other suggestions if you believe a different name would 
improve clarity.

Btw, bridge/toshiba,tc358768.yaml, which performs a similar function, 
also uses the term data-lines.

Regards,
Wadim

> 
> I don't know much about RGB parallel port to advice how this should be
> called. Can you describe the hardware more?
> 
>> +                $ref: /schemas/types.yaml#/definitions/uint32
>> +                enum: [ 16, 18, 24 ]
>> +
>>         port@1:
>>           $ref: /schemas/graph.yaml#/properties/port
>>           description: HDMI output port
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Oct. 3, 2024, 1:26 p.m. UTC | #3
On 03/10/2024 13:56, Wadim Egorov wrote:
> 
> 
> Am 03.10.24 um 12:03 schrieb Krzysztof Kozlowski:
>> On 03/10/2024 10:20, Wadim Egorov wrote:
>>> The SI9022 HDMI transmitter can be configured with 16, 18, or 24 input
>>> data lines. This commit introduces the data-lines property to the input
>>
>> lines? lanes? What are lines? like pins?
> 
> Yes, "lines" in this context refers to the number of pins used for the 
> input pixel data bus, which can support 16, 18, or 24-bit wide data 
> buses. These are parallel data lines (or pins) that carry uncompressed 
> digital video to the HDMI transmitter.
> 
>>
>>> endpoint, specifying the number of parallel RGB input pins connected
>>> to the transmitter.
>>>
>>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
>>> ---
>>>   .../bindings/display/bridge/sil,sii9022.yaml        | 13 ++++++++++++-
>>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
>>> index 5a69547ad3d7..24306f8eb107 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
>>> +++ b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
>>> @@ -81,9 +81,20 @@ properties:
>>>   
>>>       properties:
>>>         port@0:
>>> -        $ref: /schemas/graph.yaml#/properties/port
>>> +        unevaluatedProperties: false
>>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>>>           description: Parallel RGB input port
>>>   
>>> +        properties:
>>> +          endpoint:
>>> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>> +            unevaluatedProperties: false
>>> +
>>> +            properties:
>>> +              data-lines:
>>
>> No, this will confuse everyone. Considering lack of description how
>> anyone would figure out what this means?
> 
> I guess from working with the hardware/reference manual and using this chip?
> 
> I don't think it is overly confusing, especially since the port is 
> already described as the "Parallel RGB input port" which clearly implies 
> the use of pins for data transmission.


I am surprised you do not find data-lanes and data-lines confusing. For
non-native English speakers this even might sound the same.

You used earlier pins and bits, so maybe it's the same as bus-width,
which is already used all over the bindings, including one of the bridges.

Anyway a generic property should go to video-interfaces.

> 
> I am open to other suggestions if you believe a different name would 
> improve clarity.
> 
> Btw, bridge/toshiba,tc358768.yaml, which performs a similar function, 
> also uses the term data-lines.

Then this has to go to common schema.

Oh, wait, video-interfaces already have it!

Best regards,
Krzysztof
Wadim Egorov Oct. 4, 2024, 5:38 a.m. UTC | #4
Am 03.10.24 um 15:26 schrieb Krzysztof Kozlowski:
> On 03/10/2024 13:56, Wadim Egorov wrote:
>>
>>
>> Am 03.10.24 um 12:03 schrieb Krzysztof Kozlowski:
>>> On 03/10/2024 10:20, Wadim Egorov wrote:
>>>> The SI9022 HDMI transmitter can be configured with 16, 18, or 24 input
>>>> data lines. This commit introduces the data-lines property to the input
>>>
>>> lines? lanes? What are lines? like pins?
>>
>> Yes, "lines" in this context refers to the number of pins used for the
>> input pixel data bus, which can support 16, 18, or 24-bit wide data
>> buses. These are parallel data lines (or pins) that carry uncompressed
>> digital video to the HDMI transmitter.
>>
>>>
>>>> endpoint, specifying the number of parallel RGB input pins connected
>>>> to the transmitter.
>>>>
>>>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
>>>> ---
>>>>    .../bindings/display/bridge/sil,sii9022.yaml        | 13 ++++++++++++-
>>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
>>>> index 5a69547ad3d7..24306f8eb107 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
>>>> @@ -81,9 +81,20 @@ properties:
>>>>    
>>>>        properties:
>>>>          port@0:
>>>> -        $ref: /schemas/graph.yaml#/properties/port
>>>> +        unevaluatedProperties: false
>>>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>>>>            description: Parallel RGB input port
>>>>    
>>>> +        properties:
>>>> +          endpoint:
>>>> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>>> +            unevaluatedProperties: false
>>>> +
>>>> +            properties:
>>>> +              data-lines:
>>>
>>> No, this will confuse everyone. Considering lack of description how
>>> anyone would figure out what this means?
>>
>> I guess from working with the hardware/reference manual and using this chip?
>>
>> I don't think it is overly confusing, especially since the port is
>> already described as the "Parallel RGB input port" which clearly implies
>> the use of pins for data transmission.
> 
> 
> I am surprised you do not find data-lanes and data-lines confusing. For
> non-native English speakers this even might sound the same.
> 
> You used earlier pins and bits, so maybe it's the same as bus-width,
> which is already used all over the bindings, including one of the bridges.

Thanks, the bus-width property seems to be a better fit here. I'll 
rework my patches.

> 
> Anyway a generic property should go to video-interfaces.
> 
>>
>> I am open to other suggestions if you believe a different name would
>> improve clarity.
>>
>> Btw, bridge/toshiba,tc358768.yaml, which performs a similar function,
>> also uses the term data-lines.
> 
> Then this has to go to common schema.
> 
> Oh, wait, video-interfaces already have it!
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
index 5a69547ad3d7..24306f8eb107 100644
--- a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml
@@ -81,9 +81,20 @@  properties:
 
     properties:
       port@0:
-        $ref: /schemas/graph.yaml#/properties/port
+        unevaluatedProperties: false
+        $ref: /schemas/graph.yaml#/$defs/port-base
         description: Parallel RGB input port
 
+        properties:
+          endpoint:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            unevaluatedProperties: false
+
+            properties:
+              data-lines:
+                $ref: /schemas/types.yaml#/definitions/uint32
+                enum: [ 16, 18, 24 ]
+
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description: HDMI output port