diff mbox series

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

Message ID 20240531204339.277848-1-marex@denx.de (mailing list archive)
State New, archived
Headers show
Series [1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis | expand

Commit Message

Marek Vasut May 31, 2024, 8:42 p.m. UTC
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>
---
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@gmail.com>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: kernel@dh-electronics.com
---
 .../display/bridge/toshiba,tc358767.yaml       | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Alexander Stein June 4, 2024, 9:42 a.m. UTC | #1
Hi Marek,

Am Freitag, 31. Mai 2024, 22:42:03 CEST schrieb Marek Vasut:
> 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>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: kernel@dh-electronics.com
> ---
>  .../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..dcf56e996ee22 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.

Is this a property of the port or the endpoint?

> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                minItems: 2
> +                maxItems: 2
> +                items:
> +                  enum:
> +                    - 0 # -6dB de-emphasis
> +                    - 1 # -3.5dB de-emphasis
> +                    - 2 # No de-emphasis

Is there a reason you reversed the notation here? de-emphasis <-> pre-emphasis.
Commit message also says:
> in range 0=0dB, 1=3.5dB, 2=6dB .

Best regards,
Alexander

> +
>      oneOf:
>        - required:
>            - port@0
>
Marek Vasut June 4, 2024, 4:31 p.m. UTC | #2
On 6/4/24 11:42 AM, Alexander Stein wrote:
> Hi Marek,

Hi,

>> --- 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.
> 
> Is this a property of the port or the endpoint?

Endpoint I think.

>> +                $ref: /schemas/types.yaml#/definitions/uint32-array
>> +                minItems: 2
>> +                maxItems: 2
>> +                items:
>> +                  enum:
>> +                    - 0 # -6dB de-emphasis
>> +                    - 1 # -3.5dB de-emphasis
>> +                    - 2 # No de-emphasis
> 
> Is there a reason you reversed the notation here? de-emphasis <-> pre-emphasis.
> Commit message also says:
>> in range 0=0dB, 1=3.5dB, 2=6dB .

Copy-paste error from 
Documentation/devicetree/bindings/usb/snps,dwc3.yaml , fixed in V2, thanks .
Rob Herring (Arm) June 5, 2024, 4:03 p.m. UTC | #3
On Fri, May 31, 2024 at 10:42:03PM +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>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: kernel@dh-electronics.com
> ---
>  .../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..dcf56e996ee22 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:

You didn't test adding this property. You will find it isn't allowed. 
That's because 'properties/port' schema above doesn't allow extra 
properties (on the port and endpoint).

> +                description:
> +                  Display port output Pre-Emphasis settings for both ports.

Both ports? This schema is for just port 2.

> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                minItems: 2
> +                maxItems: 2
> +                items:
> +                  enum:
> +                    - 0 # -6dB de-emphasis
> +                    - 1 # -3.5dB de-emphasis
> +                    - 2 # No de-emphasis
> +
>      oneOf:
>        - required:
>            - port@0
> -- 
> 2.43.0
>
Rob Herring (Arm) June 5, 2024, 4:05 p.m. UTC | #4
On Tue, Jun 04, 2024 at 11:42:31AM +0200, Alexander Stein wrote:
> Hi Marek,
> 
> Am Freitag, 31. Mai 2024, 22:42:03 CEST schrieb Marek Vasut:
> > 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>
> > ---
> > Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> > Cc: Conor Dooley <conor+dt@kernel.org>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: David Airlie <airlied@gmail.com>
> > Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> > Cc: Jonas Karlman <jonas@kwiboo.se>
> > Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Neil Armstrong <neil.armstrong@linaro.org>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Robert Foss <rfoss@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: devicetree@vger.kernel.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: kernel@dh-electronics.com
> > ---
> >  .../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..dcf56e996ee22 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.
> 
> Is this a property of the port or the endpoint?

What's the difference? Either is the same data path.

The preference is custom properties in endpoint node, not port nodes.

Rob
Marek Vasut June 5, 2024, 4:30 p.m. UTC | #5
On 6/5/24 6:03 PM, Rob Herring wrote:

Hi,

>> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>> index 2ad0cd6dd49e0..dcf56e996ee22 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:
> 
> You didn't test adding this property. You will find it isn't allowed.
> That's because 'properties/port' schema above doesn't allow extra
> properties (on the port and endpoint).

Fixed

>> +                description:
>> +                  Display port output Pre-Emphasis settings for both ports.
> 
> Both ports? This schema is for just port 2.

'lanes' is better here. The array can be used to configure each DP main 
link lane separately, the chip supports two lanes.
Marek Vasut June 5, 2024, 4:33 p.m. UTC | #6
On 6/5/24 6:05 PM, Rob Herring wrote:

>>> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>>> index 2ad0cd6dd49e0..dcf56e996ee22 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.
>>
>> Is this a property of the port or the endpoint?
> 
> What's the difference? Either is the same data path.
> 
> The preference is custom properties in endpoint node, not port nodes.

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.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
index 2ad0cd6dd49e0..dcf56e996ee22 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.
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                minItems: 2
+                maxItems: 2
+                items:
+                  enum:
+                    - 0 # -6dB de-emphasis
+                    - 1 # -3.5dB de-emphasis
+                    - 2 # No de-emphasis
+
     oneOf:
       - required:
           - port@0