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

Conor Dooley
2024-06-23T17:20:46Z

[-- Attachment #1: Type: text/plain, Size: 3262 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]