Re: [PATCH v2 1/2] dt-bindings: drm/bridge: anx7625: add DPI flag and swing setting

[Xin Ji]

On Wed, Jan 20, 2021 at 04:57:56PM +0800, Nicolas Boichat wrote:
> On Tue, Jan 12, 2021 at 4:59 PM Xin Ji <xji@xxxxxxxxxxxxxxxx> wrote:
> >
> > Hi Rob Herring, thanks for the comments.
> >
> > On Mon, Jan 11, 2021 at 04:14:35PM -0600, Rob Herring wrote:
> > > On Thu, Dec 31, 2020 at 10:21:12AM +0800, Xin Ji wrote:
> > > > Add DPI flag for distinguish MIPI input signal type, DSI or DPI. Add
> > > > swing setting for adjusting DP tx PHY swing
> > > >
> > > > Signed-off-by: Xin Ji <xji@xxxxxxxxxxxxxxxx>
> > > > ---
> > > >  .../bindings/display/bridge/analogix,anx7625.yaml  | 25 ++++++++++++++++++++--
> > > >  1 file changed, 23 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > index 60585a4..4eb0ea3 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > @@ -34,6 +34,16 @@ properties:
> > > >      description: used for reset chip control, RESET_N pin B7.
> > > >      maxItems: 1
> > > >
> > > > +  analogix,swing-setting:
> > > > +    type: uint8-array
> > >
> > > Humm, this should have be rejected by the meta-schema.
> > We needs define an array to adjust DP tx PHY swing, the developer hopes these
> > settings are changeable, so I moved the register data to DT. Can you
> > give me some suggestion if it is rejected by the meta-schema?
> > >
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > >
> > > This is how types are defined other than boolean or nodes (object).
> > >
> > > > +    description: an array of swing register setting for DP tx PHY
> > > > +
> > > > +  analogix,mipi-dpi-in:
> > > > +    type: int
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description: indicate the MIPI rx signal type is DPI or DSI
> > >
> > > Why does this need to be in DT, you should be able to determine this
> > > based on what you are connected to.
> > As the anx7625 can receive MIPI DSI and DPI data (depends on hardware
> > implement, we have a project which have two anx7625, one is DSI input,
> > the other is DPI input), we needs to let driver know what kind of MIPI
> > rx signal input. And there is no other way to tell driver the MIPI rx
> > signal type, we needs define this flag.
> > >
> > > > +
> > > >    ports:
> > > >      type: object
> > > >
> > > > @@ -49,8 +59,8 @@ properties:
> > > >            Video port for panel or connector.
> > > >
> > > >      required:
> > > > -        - port@0
> > > > -        - port@1
> > > > +      - port@0
> > > > +      - port@1
> > > >
> > > >  required:
> > > >    - compatible
> > > > @@ -72,6 +82,17 @@ examples:
> > > >              reg = <0x58>;
> > > >              enable-gpios = <&pio 45 GPIO_ACTIVE_HIGH>;
> > > >              reset-gpios = <&pio 73 GPIO_ACTIVE_HIGH>;
> > > > +            analogix,swing-setting = <0x00 0x14>, <0x01 0x54>,
> > > > +                <0x02 0x64>, <0x03 0x74>, <0x04 0x29>,
> > > > +                <0x05 0x7b>, <0x06 0x77>, <0x07 0x5b>,
> > > > +                <0x08 0x7f>, <0x0c 0x20>, <0x0d 0x60>,
> > > > +                <0x10 0x60>, <0x12 0x40>, <0x13 0x60>,
> > > > +                <0x14 0x14>, <0x15 0x54>, <0x16 0x64>,
> > > > +                <0x17 0x74>, <0x18 0x29>, <0x19 0x7b>,
> > > > +                <0x1a 0x77>, <0x1b 0x5b>, <0x1c 0x7f>,
> > > > +                <0x20 0x20>, <0x21 0x60>, <0x24 0x60>,
> > > > +                <0x26 0x40>, <0x27 0x60>;
> > >
> > > This is a matrix, which is different from an array type.
> > OK, I'll change to array if this vendor define has been accepted.
> 
> I also wonder if we want to split the parameters per lane, and simply
> have a flat array (instead of reg/value pairs like you have now).
> 
> Registers 0x00 to 0x13 have to do with Lane 0 (and 0x14 to 0x27 with
> Lane 1): you can almost tell from the example values, they repeat. I'm
> not sure if there's any value splitting those further (I don't think
> anybody will be able to tune those without ANX's help).
> 
> So, maybe something like:
> anx,swing-setting = <<[reg values for lane 0]>, <[reg values for lane 1]>>
> where <[reg values for lane 0]> would be something like <0x14, 0x54,
> 0x64, ...> (that is, all the values between 0x00 and 0x13).
Hi Nicolas, it is a good way, 0x00 to 0x13 for lane0, 0x14 to 0x27 for lane1.
I'll split the parameters per lane.

Thanks,
Xin
> 
> > >
> > > > +            analogix,mipi-dpi-in = <0>;
> > > >
> > > >              ports {
> > > >                  #address-cells = <1>;
> > > > --
> > > > 2.7.4
> > > >