From: Wolfram Sang wsa+renesas@sang-engineering.com
The interrupts for EDID_READY or DDC_ERROR were never enabled in this driver, so reading EDID always timed out when the chip was powered down and interrupts were used. Fix this and remove clearing the interrupt flags, since they are cleared in POWER_DOWN mode anyhow (according to docs and my tests).
Signed-off-by: Wolfram Sang wsa+renesas@sang-engineering.com ---
With this patch, I can read EDID in power-down mode reliably with my Lager board. Tried with and without interrupts with two different monitors. I think this patch should go to stable.
Note that I could not yet read EDID with Magnus' Koelsch. However, the registers show that the interrupt flag and interrupt enable flag are both correctly set (they were not before this patch). I assume a pinmuxing problem or similar.
drivers/gpu/drm/i2c/adv7511.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c index 00416f23b5cb5f..85e994796d96a4 100644 --- a/drivers/gpu/drm/i2c/adv7511.c +++ b/drivers/gpu/drm/i2c/adv7511.c @@ -362,12 +362,19 @@ static void adv7511_power_on(struct adv7511 *adv7511) { adv7511->current_edid_segment = -1;
- regmap_write(adv7511->regmap, ADV7511_REG_INT(0), - ADV7511_INT0_EDID_READY); - regmap_write(adv7511->regmap, ADV7511_REG_INT(1), - ADV7511_INT1_DDC_ERROR); regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, ADV7511_POWER_POWER_DOWN, 0); + if (adv7511->i2c_main->irq) { + /* + * Documentation says the INT_ENABLE registers are reset in + * POWER_DOWN mode. My tests with a 7511w show something else + * but let's stick to the documentation. + */ + regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), + ADV7511_INT0_EDID_READY); + regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), + ADV7511_INT1_DDC_ERROR); + }
/* * Per spec it is allowed to pulse the HDP signal to indicate that the @@ -567,12 +574,14 @@ static int adv7511_get_modes(struct drm_encoder *encoder,
/* Reading the EDID only works if the device is powered */ if (!adv7511->powered) { - regmap_write(adv7511->regmap, ADV7511_REG_INT(0), - ADV7511_INT0_EDID_READY); - regmap_write(adv7511->regmap, ADV7511_REG_INT(1), - ADV7511_INT1_DDC_ERROR); regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, ADV7511_POWER_POWER_DOWN, 0); + if (adv7511->i2c_main->irq) { + regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), + ADV7511_INT0_EDID_READY); + regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), + ADV7511_INT1_DDC_ERROR); + } adv7511->current_edid_segment = -1; }
Hi Wolfram,
On Wed, Nov 25, 2015 at 6:05 AM, Wolfram Sang wsa@the-dreams.de wrote:
Note that I could not yet read EDID with Magnus' Koelsch.
This has been tackled as well now. The clock for the GPIO controller was off, so no interrupt was passed through.
Thanks a lot for your efforts! When you have time, can you please show me the patch that fixes that GPIO interrupt problem? I'm asking because I may have ran into a similar issue on Gose or Alt.
Cheers,
/ magnus
This has been tackled as well now. The clock for the GPIO controller was off, so no interrupt was passed through.
Thanks a lot for your efforts! When you have time, can you please show me the patch that fixes that GPIO interrupt problem? I'm asking because I may have ran into a similar issue on Gose or Alt.
For Koelsch, this simply meant I reverted Geert's clocks-off patch. But there is a fundamental problem below: GPIO interrupts described with the "interrupts" property in DT do not get the underlying GPIO requested. And if noone else is using the GPIO block, its clock may be turned off.
I have an idea how to fix it, need to check priorities of the other tasks, though.
BTW, the continous EDID read test on your board is currently at >360000 successful reads, a not a single failure :)
Hi Wolfram,
On Wed, Nov 25, 2015 at 3:48 PM, Wolfram Sang wsa@the-dreams.de wrote:
This has been tackled as well now. The clock for the GPIO controller was off, so no interrupt was passed through.
Thanks a lot for your efforts! When you have time, can you please show me the patch that fixes that GPIO interrupt problem? I'm asking because I may have ran into a similar issue on Gose or Alt.
For Koelsch, this simply meant I reverted Geert's clocks-off patch. But there is a fundamental problem below: GPIO interrupts described with the "interrupts" property in DT do not get the underlying GPIO requested. And if noone else is using the GPIO block, its clock may be turned off.
I have an idea how to fix it, need to check priorities of the other tasks, though.
I guess you mean that the GPIO callbacks include Runtime PM handling however for irq_chip Runtime PM may not be hooked up so the GPIO block is in such case is not powered on / get clock enabled?
BTW, the continous EDID read test on your board is currently at >360000 successful reads, a not a single failure :)
Excellent, thank you!
/ magnus
I guess you mean that the GPIO callbacks include Runtime PM handling however for irq_chip Runtime PM may not be hooked up so the GPIO block is in such case is not powered on / get clock enabled?
Yes. There is another drawback when GPIOs are not properly requested. It is still possible to request them from userspace although a kernel driver is using them. I am playing with the idea that the GPIO core auto-requests GPIOs which are not already requested but still set up as interrupts.
On 11/25/2015 09:27 AM, Wolfram Sang wrote:
I guess you mean that the GPIO callbacks include Runtime PM handling however for irq_chip Runtime PM may not be hooked up so the GPIO block is in such case is not powered on / get clock enabled?
Yes. There is another drawback when GPIOs are not properly requested. It is still possible to request them from userspace although a kernel driver is using them. I am playing with the idea that the GPIO core auto-requests GPIOs which are not already requested but still set up as interrupts.
I think the GPIO core already reserves the pins that are requested as IRQs. See gpiochip_lock_as_irq().
As for PM see this discussion http://lkml.iu.edu/hypermail/linux/kernel/1511.1/01645.html
- Lars
On Thu, Nov 26, 2015 at 01:43:33PM +0100, Lars-Peter Clausen wrote:
On 11/25/2015 09:27 AM, Wolfram Sang wrote:
I guess you mean that the GPIO callbacks include Runtime PM handling however for irq_chip Runtime PM may not be hooked up so the GPIO block is in such case is not powered on / get clock enabled?
Yes. There is another drawback when GPIOs are not properly requested. It is still possible to request them from userspace although a kernel driver is using them. I am playing with the idea that the GPIO core auto-requests GPIOs which are not already requested but still set up as interrupts.
I think the GPIO core already reserves the pins that are requested as IRQs. See gpiochip_lock_as_irq().
I could still export them via sysfs. They were also not showing up in /sys/kernel/debug/gpio.
As for PM see this discussion http://lkml.iu.edu/hypermail/linux/kernel/1511.1/01645.html
Nice! Thanks for this pointer.
dri-devel@lists.freedesktop.org
-
Lars-Peter Clausen -
Magnus Damm -
Wolfram Sang