Re: [PATCH v3 4/4] arm64: dts: renesas: r8a77995: draak: Enable HDMI display output

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hello,

On Fri, Jun 18, 2021 at 02:07:48PM +0200, Geert Uytterhoeven wrote:
> On Fri, Jun 18, 2021 at 2:01 PM Ulrich Hecht wrote:
> > > On 06/18/2021 10:05 AM Geert Uytterhoeven wrote:
> > > On Tue, Sep 25, 2018 at 6:34 PM Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com> wrote:
> > > > From: Ulrich Hecht <uli+renesas@fpond.eu>
> > > >
> > > > Adds LVDS decoder, HDMI encoder and connector for the Draak board.
> > > >
> > > > The LVDS0 and LVDS1 encoders can use the DU_DOTCLKIN0, DU_DOTCLKIN1 and
> > > > EXTAL externals clocks. Two of them are provided to the SoC on the Draak
> > > > board, hook them up in DT.
> > > >
> > > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > > Tested-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > >
> > > > --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > > +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >
> > > > @@ -190,6 +225,43 @@
> > > >
> > > >         };
> > > >
> > > > +       hdmi-encoder@39 {
> > > > +               compatible = "adi,adv7511w";
> > > > +               reg = <0x39>, <0x3f>, <0x38>, <0x3c>;
> > > > +               reg-names = "main", "edid", "packet", "cec";
> > > > +               interrupt-parent = <&gpio1>;
> > > > +               interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
> > > > +
> > > > +               /* Depends on LVDS */
> > > > +               max-clock = <135000000>;
> > > > +               min-vrefresh = <50>;
> > >
> > > Where do these two come from? They fail to validate with commit
> > > cfe34bb7a770c5d8 ("dt-bindings: drm: bridge: adi,adv7511.txt: convert
> > > to yaml").
> > > I can't find where it is used in the driver, nor in the driver history.
> >
> > I have found a prototype patch in my archives that uses these
> > properties. I guess the patch itself didn't make it into the final
> > series, but the properties inadvertently did. I vaguely remember
> > this was supposed to work around an issue with modes that use a
> > higher clock than supported by one of the parts in the display
> > pipeline.
> 
> Thanks, I already suspected something like that...

Sounds like a BSP attempt to model limitations of the DU and/or the PCB
and implement them in the adv7511 driver. There's similar code in the
VGA encoder driver that really doesn't belong there.

> > I would say that if there are no issues with HDMI output, both the
> > patch and the properties are obsolete.
> 
> Anyone with a Draak to verify?

I don't carry Ulrich's patch in my branch, and last time I checked, HDMI
output was functional. Do you want me to retest ?

-- 
Regards,

Laurent Pinchart