Received-SPF: pass (google.com: domain of linux-kernel+bounces-129085-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240402105925.905144-1-alex@shruggie.ro> <20240402165307.GA31874@francesco-nb>
In-Reply-To: <20240402165307.GA31874@francesco-nb>
From: Alexandru Ardelean <alex@shruggie.ro>
Date: Wed, 3 Apr 2024 09:32:41 +0300
Message-ID: <CAH3L5Qr-sT+Q9ZvNSxHKwVMr8-3fU5WPvvaccEWWH49x7oWMyQ@mail.gmail.com>
Subject: Re: [PATCH 1/2] drm/bridge: lt8912b: add support for P/N pin swap
To: Francesco Dolcini <francesco@dolcini.it>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, 
	devicetree@vger.kernel.org, adrien.grassein@gmail.com, 
	andrzej.hajda@intel.com, neil.armstrong@linaro.org, rfoss@kernel.org, 
	Laurent.pinchart@ideasonboard.com, jonas@kwiboo.se, jernej.skrabec@gmail.com, 
	airlied@gmail.com, daniel@ffwll.ch, maarten.lankhorst@linux.intel.com, 
	mripard@kernel.org, tzimmermann@suse.de, robh@kernel.org, 
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, 
	stefan.eichenberger@toradex.com, francesco.dolcini@toradex.com, 
	marius.muresan@mxt.ro, irina.muresan@mxt.ro
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Apr 2, 2024 at 7:53=E2=80=AFPM Francesco Dolcini <francesco@dolcini=
it> wrote:
>
> Hello Alexandru, thanks for your patch.
>
> On Tue, Apr 02, 2024 at 01:59:24PM +0300, Alexandru Ardelean wrote:
> > On some HW designs, it's easier for the layout if the P/N pins are swap=
ped.
> > In those cases, we need to adjust (for this) by configuring the MIPI an=
alog
> > registers differently. Specifically, register 0x3e needs to be 0xf6
> > (instead of 0xd6).
> >
> > This change adds a 'lontium,pn-swap' device-tree property to configure =
the
> > MIPI analog registers for P/N swap.
> >
> > Signed-off-by: Alexandru Ardelean <alex@shruggie.ro>
> > ---
> >  drivers/gpu/drm/bridge/lontium-lt8912b.c | 25 +++++++++++++++++++++---
> >  1 file changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm=
/bridge/lontium-lt8912b.c
> > index 4b2ae27f0a57f..154126bb922b4 100644
> > --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> > +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> > @@ -47,6 +47,7 @@ struct lt8912 {
> >
> >       u8 data_lanes;
> >       bool is_power_on;
> > +     bool do_pn_swap;
> >  };
> >
> >  static int lt8912_write_init_config(struct lt8912 *lt)
> > @@ -78,15 +79,31 @@ static int lt8912_write_init_config(struct lt8912 *=
lt)
> >               {0x55, 0x44},
> >               {0x57, 0x01},
> >               {0x5a, 0x02},
> > -
> > -             /*MIPI Analog*/
> > +     };
> > +     const struct reg_sequence mipi_analog_seq[] =3D {
> >               {0x3e, 0xd6},
> >               {0x3f, 0xd4},
> >               {0x41, 0x3c},
> >               {0xB2, 0x00},
> >       };
> > +     const struct reg_sequence mipi_analog_pn_swap_seq[] =3D {
> > +             {0x3e, 0xf6},
> > +             {0x3f, 0xd4},
> > +             {0x41, 0x3c},
> > +             {0xB2, 0x00},
> > +     };
> > +     int ret;
> >
> > -     return regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_SI=
ZE(seq));
> > +     ret =3D regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_S=
IZE(seq));
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     if (!lt->do_pn_swap)
> > +             return regmap_multi_reg_write(lt->regmap[I2C_MAIN], mipi_=
analog_seq,
> > +                                           ARRAY_SIZE(mipi_analog_seq)=
);
> > +
> > +     return regmap_multi_reg_write(lt->regmap[I2C_MAIN], mipi_analog_p=
n_swap_seq,
> > +                                   ARRAY_SIZE(mipi_analog_pn_swap_seq)=
);
>
> Can you just remove {0x3e, 0xd6} from the register/value array and write
> it afterward depending on `do_pn_swap` value? Or keep it with the
> current value and only overwrite it when do_pn_swap is true?
>
> If you do it this way is a 4 line change.

Hmm, good point.
I did it like this, because I don't have a board with the P/N in the
0xd6 configuration, to test.
But, if I leave it like this, and just overwrite 0x3e when
`do_pn_swap` is true, I can test that; and I don't need to test the
original case.

I'm actually not 100% sure here if the order of registers (being
written) matters for the initialization.


>
>
> >  static int lt8912_write_mipi_basic_config(struct lt8912 *lt)
> > @@ -702,6 +719,8 @@ static int lt8912_parse_dt(struct lt8912 *lt)
> >       }
> >       lt->gp_reset =3D gp_reset;
> >
> > +     lt->do_pn_swap =3D device_property_read_bool(dev, "lontium,pn-swa=
p");
>
> I would call this variable the same that is called in the lontium
> documentation, mipirx_diff_swap

Oh.
I actually based this change on a reference software for the LT8912B.
I didn't get a chance to see/find a documentation for the registers.
I compared with the Linux driver, to see what was missing to get
output on the HDMI display (for our setup).

>
> Francesco
>