Re: [PATCH v5 3/3] arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle

From: jacopo mondi
Date: Fri Mar 16 2018 - 05:40:24 EST


Hi Niklas,
thanks for review,

On Fri, Mar 16, 2018 at 01:00:04AM +0100, Niklas SÃderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> This one must depend on '[PATCH v2 0/5] arm64: dts: renesas: r8a77970:
> enable HDMI output' or something similar not yet in renesas-drivers
> repository correct?

Yes, I have listed dependencies of this series in my v1 cover letter:

---------------------------------------------------------------------------
The series is based on Laurent's drm/next/du branch with patches on top for:
- Sergei: Enable PFC, I2c, GPIOs for r8a77970
- Sergei: Add support for r8a77970 in DU and add display device nodes in
r8a77970 DTSI
- Niklas: Connect DU LVDS output to HDMI bridge adv7511w in Eagle DTS
- Sergei: fix video output on R8A77970

A base branch with these patches applied is available at
git://jmondi.org/linux v3m/v4.16-rc3/base
---------------------------------------------------------------------------

My bad I have not reported this in all cover letters (and I have not
reported the full name of the series)

PFC, GPIO and I2c support I have listed as dependencies seems to have
landed in renesas-drivers, while I don't see yet:

Sergei:
[PATCH v2 0/5] Add R8A77970/V3MSK LVDS/HDMI support

whose patches for r8a77970 dtsi are included in yours:
[PATCH v2 0/5] arm64: dts: renesas: r8a77970: enable HDMI output

On top I also have Sergei's/Laurent's:
[PATCH v4] v4l: vsp1: Fix video output on R8A77970

>
> In the next version would you care to include the LVDS commit from the
> dependency series and squash this change into that one or in some other
> good manger stack to two? Laurent told me he did not like 5/5 in that
> patch-set as it did not yet have the LVDS decoder node due to no driver
> existed at that time when I posted that even if it's not strictly needed
> to get the display working :-)


I'll let Simon suggest how he preferes to handle this, if he wants me
to re-submit your series with this patch squashed on top or he prefers
to deal with this himself.
>
> I also think you should split this last patch out to a separate series
> as it should go in Simon's tree while the driver and documentation is
> going in earlier in a different tree right?

I assume bindings and driver go through DRM and Simon is to pick up
the Eagle changes.

>
> On a side note, do you plan to update the Gen2 boards DTS files which
> also have a decoder which are not yet described in DT?

Actually I'm not aware of Gen2 boards with this chip and similar
display pipelines. Can you point me to which one needs to have its
DTS brushed?

Thanks
j

>
> On 2018-03-15 17:11:56 +0100, Jacopo Mondi wrote:
> > The R-Car V3M Eagle board includes a transparent THC63LVD1024 LVDS
> > decoder, connected to the on-chip LVDS encoder output on one side
> > and to HDMI encoder ADV7511w on the other one.
> >
> > As the decoder does not need any configuration it has been so-far
> > omitted from DTS. Now that a driver is available, describe it in DT
> > as well.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> > Reviewed-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> > ---
> > arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 33 +++++++++++++++++++++++---
> > 1 file changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> > index c0fd144..69f43b8 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> > @@ -42,6 +42,33 @@
> > };
> > };
> > };
> > +
> > + thc63lvd1024: lvds-decoder {
> > + compatible = "thine,thc63lvd1024";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > +
> > + thc63lvd1024_in_0: endpoint {
> > + remote-endpoint = <&lvds0_out>;
> > + };
> > + };
> > +
> > + port@2{
> > + reg = <2>;
> > +
> > + thc63lvd1024_out_2: endpoint {
> > + remote-endpoint = <&adv7511_in>;
> > + };
> > +
> > + };
> > +
> > + };
> > + };
> > };
> >
> > &avb {
> > @@ -98,7 +125,7 @@
> > port@0 {
> > reg = <0>;
> > adv7511_in: endpoint {
> > - remote-endpoint = <&lvds0_out>;
> > + remote-endpoint = <&thc63lvd1024_out_2>;
> > };
> > };
> >
> > @@ -152,8 +179,8 @@
> >
> > ports {
> > port@1 {
> > - endpoint {
> > - remote-endpoint = <&adv7511_in>;
> > + lvds0_out: endpoint {
> > + remote-endpoint = <&thc63lvd1024_in_0>;
> > };
> > };
> > };
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas SÃderlund

Attachment: signature.asc
Description: PGP signature