linux-kernel - Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABkfQAGcSsQ74FtvAK4_awHRXswgBrThKww_xhpmTzordZ5X8w@mail.gmail.com>
Date:   Tue, 23 Mar 2021 12:08:58 +0100
From:   Adrien Grassein <adrien.grassein@...il.com>
To:     Robert Foss <robert.foss@...aro.org>
Cc:     David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Rob Herring <robh+dt@...nel.org>,
        Neil Armstrong <narmstrong@...libre.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge

Le mar. 23 mars 2021 à 11:42, Robert Foss <robert.foss@...aro.org> a écrit :
>
> On Tue, 23 Mar 2021 at 11:01, Adrien Grassein <adrien.grassein@...il.com> wrote:
> >
> > Hey Robert,
> >
> > Thanks for the update.
> >
> > Le mar. 23 mars 2021 à 10:10, Robert Foss <robert.foss@...aro.org> a écrit :
> > >
> > > Hey Adrien,
> > >
> > > Sorry about the slow reply, but I just received the documentation from
> > > the vendor. So let's dig in to the HPD issue.
> >
> > No problem :)
> > >
> > > > +static enum drm_connector_status lt8912_check_cable_status(struct lt8912 *lt)
> > > > +{
> > > > +       int ret;
> > > > +       unsigned int reg_val;
> > > > +
> > > > +       ret = regmap_read(lt->regmap[I2C_MAIN], 0xC1, &reg_val);
> > > > +       if (ret)
> > > > +               return connector_status_unknown;
> > > > +
> > > > +       if (reg_val & BIT(7))
> > > > +               return connector_status_connected;
> > >
> > > Register 0xc0 & BIT(7) - HPD signal after debounce
> > > Register 0xc0 & BIT(6) - HPD signal for TX HPD pad
> >
> > So, if I understand well, I need to write 0xc0 & BIT(6) with 1 to
> > enable the HPD pin.
>
> Ah, sorry about being a bit terse.
>
> Both bit 6 & 7 are read only, and are probably best read after an IRQ.

In my case, IRQ is not triggered at all.
When reading the value of the HPD pin, I always get 1 (and no
transition occurs when plugging / unplugging a cable).
The HPD IRQ is done on the HDMI connector driver [5].
I think a register configuration should be done to enable the IRQ pin
or maybe there is a nug in electronics.
The HPD pin is linked to a 2.2k pullup resistor (maybe it's wrong)

>
> > >
> > > > +
> > > > +static int lt8912_probe(struct i2c_client *client,
> > > > +        const struct i2c_device_id *id)
> > > > +{
> > > > +       static struct lt8912 *lt;
> > > > +       int ret = 0;
> > > > +       struct device *dev = &client->dev;
> > > > +
> > > > +       lt = devm_kzalloc(dev, sizeof(struct lt8912), GFP_KERNEL);
> > > > +       if (!lt)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       lt->dev = dev;
> > > > +       lt->i2c_client[0] = client;
> > > > +       lt->cable_status = connector_status_unknown;
> > > > +       lt->workq = create_workqueue("lt8912_workq");
> > >
> > > Looking at [1] and maybe even better [2], I think this polling
> > > approach is the wrong way to go. And with access to documentation, I
> > > think we should be able to sort this out.
> >
> > I neither like the polling approach too. I did it to go on this issue.
> > I will totally remove it once the HPD issue will be resolved.
> > >
> > > Using the irq driver approach requires the interrupt pin to be
> > > configured. Pin 63 of the lt8912b is the IRQ output pin.
> > >
> > > In order to trigger interrupts based on it, the dt-binding would need
> > > to be updated[3][4] as well as whichever DTS you're using.
> > >
> >
> > The IRQ part is working well in my DTB. It test it by adding some
> > electronics to emulate the HPD pin on the GPIO expander where the HPD
> > pin is linked.
>
> Looking at the dt-binding patch, it does not seem to list any
> interrupts. So that should be added. I think the irq support from [3]
> & [4] can be pretty much copied.
>
> Then we can come back and replace the polling code with the IRQ driven
> code from [2].

My board uses a "max7323" GPIO expander and the HPD pin is linked to it.
I test this GPIO expander by soldering a pull up resistor and an
interrupt on it and an interrupt was correctly triggered in both
max7323 driver and hdmi-connector;
So I guess that my DTB configuration is correct.
I made my DBT configuration available:
  - hdmi-connector node: [6]
  - lt8912b node: |7]
  - max7323 node: [8].


>
> >
> > >
> > > [1] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/analogix/anx7625.c#L1751
> > >
> > > [2] https://github.com/torvalds/linux/blob/v5.11/drivers/gpu/drm/bridge/lontium-lt9611.c#L1160
> > >
> > > [3] https://github.com/torvalds/linux/blob/v5.11/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml#L27
> > >
> > > [4] https://github.com/torvalds/linux/blob/v5.11/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml#L144

[5] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/display-connector.c#L199
[6] https://github.com/grassead/linux-next/blob/master/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts#L37
[7] https://github.com/grassead/linux-next/blob/master/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts#L249
[8] https://github.com/grassead/linux-next/blob/master/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts#L291


Thanks,