Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
Alright, I think the best way forward is to remove the polled HPD support. Don't forget to update flags and function pointers. With that, feel free to add my r-b. Reviewed-by: Robert Foss
Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
> > 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.
Thanks, that makes sense. pin14 is connected directly to the physical connector.
>
> >
> > 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.
Some chips support alternative addresses, I guess we can assume that
is the case here.
>
> > > 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.
Ack. Ok, false alert with pin14 then.
> >
> > >
> > > >
> > > > > >
> > > > > > > +
> > > > > > > +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>;
> > }
Ok, so pin63 is the output from lt8912b we should be listening to.
And looking at lt9611 it seems to be low=active, so IRQ_TYPE_EDGE_FALLING.
interrupts-extended = <&max7323 $LT8912B_PIN_63 IRQ_TYPE_EDGE_FALLING>;
> >
> > 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/displa
Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
Le mar. 23 mars 2021 à 15:16, Adrien Grassein
a écrit :
>
> Le mar. 23 mars 2021 à 15:07, Robert Foss 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, ®_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 y
Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
Le mar. 23 mars 2021 à 15:07, Robert Foss 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, ®_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/maste
Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
> > > >
> > > > > +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, ®_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.
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
> 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.
>
> >
> > > >
> > > > > +
> > > > > +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
Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
Le mar. 23 mars 2021 à 11:42, Robert Foss a écrit :
>
> On Tue, 23 Mar 2021 at 11:01, Adrien Grassein
> wrote:
> >
> > Hey Robert,
> >
> > Thanks for the update.
> >
> > Le mar. 23 mars 2021 à 10:10, Robert Foss 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, ®_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,
Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
On Tue, 23 Mar 2021 at 11:01, Adrien Grassein wrote:
>
> Hey Robert,
>
> Thanks for the update.
>
> Le mar. 23 mars 2021 à 10:10, Robert Foss 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, ®_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
Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
Hey Robert,
Thanks for the update.
Le mar. 23 mars 2021 à 10:10, Robert Foss 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, ®_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.
>
> > +
> > +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.
>
> [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
Thanks a lot,
Adrien
Re: [PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
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.
> +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, ®_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
> +
> +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.
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.
[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
[PATCH v7 2/2] drm/bridge: Introduce LT8912B DSI to HDMI bridge
Lontium LT8912B is a DSI to HDMI bridge.
Signed-off-by: Adrien Grassein
Reported-by: kernel test robot
---
MAINTAINERS | 1 +
drivers/gpu/drm/bridge/Kconfig | 14 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/lontium-lt8912b.c | 836 +++
4 files changed, 852 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/lontium-lt8912b.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 719f633c8a28..072dea9c43ed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10466,6 +10466,7 @@ LONTIUM LT8912B MIPI TO HDMI BRIDGE
M: Adrien Grassein
S: Maintained
F: Documentation/devicetree/bindings/display/bridge/lontium,lt8912b.yaml
+F: drivers/gpu/drm/bridge/lontium-lt8912b.c
LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
M: Sathya Prakash
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index e4110d6ca7b3..f2c5ec75d2f5 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -48,6 +48,20 @@ config DRM_DISPLAY_CONNECTOR
on ARM-based platforms. Saying Y here when this driver is not needed
will not cause any issue.
+config DRM_LONTIUM_LT8912B
+ tristate "Lontium LT8912B DSI/HDMI bridge"
+ depends on OF
+ select DRM_PANEL_BRIDGE
+ select DRM_KMS_HELPER
+ select REGMAP_I2C
+ help
+ Driver for Lontium LT8912B DSI to HDMI bridge
+ chip driver.
+ Please say Y if you have such hardware.
+
+ Say M here if you want to support this hardware as a module.
+ The module will be named "lontium-lt8912b".
+
config DRM_LONTIUM_LT9611
tristate "Lontium LT9611 DSI/HDMI bridge"
select SND_SOC_HDMI_CODEC if SND_SOC
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 86e7acc76f8d..756ce401aad3 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o
+obj-$(CONFIG_DRM_LONTIUM_LT8912B) += lontium-lt8912b.o
obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o
obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c
b/drivers/gpu/drm/bridge/lontium-lt8912b.c
new file mode 100644
index ..82501fe2e408
--- /dev/null
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -0,0 +1,836 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#include
+
+#define I2C_MAIN 0
+#define I2C_ADDR_MAIN 0x48
+
+#define I2C_CEC_DSI 1
+#define I2C_ADDR_CEC_DSI 0x49
+
+#define I2C_MAX_IDX 2
+
+#define HPD_WORK_DELAY_MS 1000
+
+struct lt8912 {
+ struct device *dev;
+ struct drm_bridge bridge;
+ struct drm_connector connector;
+
+ struct i2c_client *i2c_client[I2C_MAX_IDX];
+ struct regmap *regmap[I2C_MAX_IDX];
+
+ struct device_node *host_node;
+ struct drm_bridge *hdmi_port;
+
+ struct mipi_dsi_device *dsi;
+
+ struct gpio_desc *gp_reset;
+
+ struct videomode mode;
+
+ u8 data_lanes;
+ bool is_power_on;
+ bool is_attached;
+
+ enum drm_connector_status cable_status;
+ struct workqueue_struct *workq;
+ struct delayed_work lt8912_check_hpd_work_id;
+};
+
+static int lt8912_write_init_config(struct lt8912 *lt)
+{
+ const struct reg_sequence seq[] = {
+ /* Digital clock en*/
+ {0x08, 0xff},
+ {0x09, 0xff},
+ {0x0a, 0xff},
+ {0x0b, 0x7c},
+ {0x0c, 0xff},
+ {0x42, 0x04},
+
+ /*Tx Analog*/
+ {0x31, 0xb1},
+ {0x32, 0xb1},
+ {0x33, 0x0e},
+ {0x37, 0x00},
+ {0x38, 0x22},
+ {0x60, 0x82},
+
+ /*Cbus Analog*/
+ {0x39, 0x45},
+ {0x3a, 0x00},
+ {0x3b, 0x00},
+
+ /*HDMI Pll Analog*/
+ {0x44, 0x31},
+ {0x55, 0x44},
+ {0x57, 0x01},
+ {0x5a, 0x02},
+
+ /*MIPI Analog*/
+ {0x3e, 0xd6},
+ {0x3f, 0xd4},
+ {0x41, 0x3c},
+ {0xB2, 0x00},
+ };
+
+ return regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq,
ARRAY_SIZE(seq));
+}
+
+static int lt8912_write_mipi_basic_config(struct lt8912 *lt)
+{
+ const struct reg_sequence seq[] = {
+ {0x12, 0x04},
+ {0x14, 0x00},
+ {0x15, 0x00},
+ {0x1a, 0x03},
+ {0x1b, 0x03},
+ };
+
+ return reg
