Re: [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis

Marek Vasut
2024-06-24T00:36:10Z

On 6/23/24 7:20 PM, Conor Dooley wrote:
> On Sun, Jun 23, 2024 at 04:48:47PM +0200, Marek Vasut wrote:
>> On 6/22/24 1:56 PM, Conor Dooley wrote:
>>> On Fri, Jun 21, 2024 at 05:53:53PM +0200, Marek Vasut wrote:
>>>> Document default DP port preemphasis configurable via new DT property
>>>> "toshiba,pre-emphasis". This is useful in case the DP link properties
>>>> are known and starting link training from preemphasis setting of 0 dB
>>>> is not useful. The preemphasis can be set separately for both DP lanes
>>>> in range 0=0dB, 1=3.5dB, 2=6dB .
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> ---
>>>> V2: - Fix the type to u8 array
>>>> - Fix the enum items to match what they represent
>>>> ---
>>>> .../display/bridge/toshiba,tc358767.yaml | 18 ++++++++++++++++++
>>>> 1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>>>> index 2ad0cd6dd49e0..6287eb2b40908 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>>>> @@ -98,6 +98,24 @@ properties:
>>>> reference to a valid eDP panel input endpoint node. This port is
>>>> optional, treated as DP panel if not defined
>>>> + properties:
>>>> + endpoint:
>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>> + unevaluatedProperties: false
>>>> +
>>>> + properties:
>>>> + toshiba,pre-emphasis:
>>>> + description:
>>>> + Display port output Pre-Emphasis settings for both ports.
>>>
>>> Why here and not in the port nodes?
>>
>> There was a short discussion about that in V1:
>>
>> https://lore.kernel.org/all/00e9ef90-3bbe-4556-8da9-462f65928781@denx.de/
>>
>> "
>> Let's keep it in the endpoint node.
>>
>> There is some mention in the TC9595 datasheet that the DP might operate
>> in some split mode, where each DP lane is used to feed one display (?),
>> so I assume in that case there might be two endpoints (?), but that is
>> not supported right now.
>>
>> If that is ever needed, I guess this array would have minItems 1 and
>> maxItems 2 and another endpoint would be added to the schema for this
>> port 2.
>> "
>
> Can this be put in the commit message please?

Will do.

>>>> + $ref: /schemas/types.yaml#/definitions/uint8-array
>>>> + minItems: 2
>>>> + maxItems: 2
>>>> + items:
>>>> + enum:
>>>> + - 0 # No pre-emphasis
>>>> + - 1 # 3.5dB pre-emphasis
>>>> + - 2 # 6dB pre-emphasis
>>>
>>> I'd love to say please make this -bB and put this in units, but that'd
>>> require it being a string..
>>
>> I can do that. Do you think that's worth it ?
>
> I dunno, I'd advocate for it for any other unit cos I would ask for the
> unit to be changed into something that didn't require fractions. But for
> decibels, that just going to be confusing given how it works. I think
> for dB it's just not worth it.

All right then.