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

[Robert Foss]

On Tue, 23 Mar 2021 at 11:01, Adrien Grassein <adrien.grassein@xxxxxxxxx> wrote:
>
> Hey Robert,
>
> Thanks for the update.
>
> Le mar. 23 mars 2021 à 10:10, Robert Foss <robert.foss@xxxxxxxxxx> 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.

> >
> > > +
> > > +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].

>
> >
> > [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