Message-ID: <CABkfQAFhVx==HiCo5ipTqTahdxWQauweGxqc7tEdCf=gGS8aGw@mail.gmail.com>
Date:   Tue, 23 Mar 2021 15:39:33 +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 à 15:16, Adrien Grassein
<adrien.grassein@...il.com> a écrit :
>
> Le mar. 23 mars 2021 à 15:07, Robert Foss <robert.foss@...aro.org> a écrit :
> >
> > > > > >
> > > > > > > +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.
> >
> > Are you saying that pin 63 never is high, or that an irq/isr routine
> > isn't getting executed?
> >
> > > 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.
> >
> > After looking at the documentation a bit more, I think we can ignore
> > pin63 and instead have a look at pin14. It is the HDMI TX HPD Control
> > pin. It has a 100k pull-down, so it should be active high.
>
> pin63 is always active high.
> pin14 is connected to the HDMI logic (pin 19 of the HDMI connector)
> with a 100k pull-down.
>
> >
> > I also found some different I2C addresses than what you've used, I
> > assume the device is available on both addresses.
> >
> > Chip control registers, address:0x90
> > CEC control registers, address 0x92
> >
> Strange, configuration seems to be working well with the address used
> in my driver.
>
> > > The HPD pin is linked to a 2.2k pullup resistor (maybe it's wrong)
> >
> > The datasheet isn't entirely clear about if pin14 has an internal 100k
> > pull-down, or if they recommend adding a 100k pull-down.
> >
> > But this does seem like an issue.
>
> pin14 can't be used directly. I guess it's used by the internal logic
> of the chip to generate the HPD (pin63) signal.
> >
> > >
> > > >
> > > > > >
> > > > > > > +
> > > > > > > +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].
> >
> > Looking at [7] I think that you would want to add something like:
> >
> > hdmi-bridge@48 {
> >         interrupts-extended = <&max7323 $LT8912B_PIN_14 IRQ_TYPE_EDGE_RISING>;
> > }
> >
> > And of course add the corresponding parts from [2] and [3].
> >
> > >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > [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,
>
> Maybe the conclusion is that we cannot have the HPD working.
>
> Thanks,

After a bit of reflexion, I think that the pin 14 may be misconnected.
As the documentation seems unclear about this pin the chip can have
bad information about the read HDMI HPD state and then not generate an
interruption.

So, I guess that there is a way that HPD will never work on my board :(.

With this taken in account, I think that polling the status is the
best (and only) solution to emulate an HPD behaviour.

Thanks again,

Powered by blists - more mailing lists