| Message ID | 20240621155430.92894-1-marex@denx.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis | expand |
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> > --- > 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 > --- > 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? > + $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.. > + > oneOf: > - required: > - port@0 > -- > 2.43.0 >
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. " >> + $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 ?
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? > > > + $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.
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.
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. + $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 + oneOf: - required: - port@0
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 --- 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(+)