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

[Conor Dooley <conor@kernel.org>]

[-- 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 --]