[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
On Fri, Nov 25, 2016 at 1:23 AM, Laurent Pinchart wrote: >> > Daniel, why do we have an API the is clearly related to interrupt handling >> > but requires the caller to implement a workqueue ? >> >> Because in general you need that workqueue anyway, and up to now there was >> no driver ever who didn't have a work-queue already. > > None of the bridge drivers in drivers/gpu/drm/bridge/ have workqueues. They > call the HPD helpers from a threaded interrupt handler though. Sleeping in > that context is fine, calling functions that might rely on interrupts from the > same device to signal completion (such as reading EDID through .get_modes()) > isn't. Hm, as long as they all use the bit-banging interfaces they'll probably be all fine. For everyone else you need multiple layers of work items to make sure you never end up stalling in an interrupt vs. holding-mode_config.mutex deadlock. So still not convinced we need this ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
Hi Daniel, On Wednesday 23 Nov 2016 08:55:37 Daniel Vetter wrote: > On Tue, Nov 22, 2016 at 08:23:38PM +0200, Laurent Pinchart wrote: > > On Tuesday 22 Nov 2016 10:07:53 John Stultz wrote: > >> On Tue, Nov 22, 2016 at 9:38 AM, John Stultz wrote: > >>> Interestingly, without the msleep added in this patch, removing the > >>> wait_event_interruptible_timeout() method in adv7511_wait_for_edid() > >>> and using the polling loop seems to make things just as reliable. So > >>> maybe something is off with the irq handling here instead? > >> > >> A.. So I think the trouble here is the that when we fail waiting > >> for the irq, the backtrace is as follows: > >> > >> [8.318654] [] dump_backtrace+0x0/0x1a0 > >> [8.318661] [] show_stack+0x14/0x20 > >> [8.318671] [] dump_stack+0x90/0xb0 > >> [8.318680] [] adv7511_get_edid_block+0x2c8/0x320 > >> [8.318687] [] drm_do_get_edid+0x78/0x280 > >> [8.318693] [] adv7511_get_modes+0x80/0xd8 > >> [8.318700] [] > >> adv7511_connector_get_modes+0x14/0x20 > >> [8.318710] [] > >> drm_helper_probe_single_connector_modes+0x2bc/0x500 > >> [8.318718] [] > >> drm_fb_helper_hotplug_event+0x130/0x188 > >> [8.318726] [] > >> drm_fbdev_cma_hotplug_event+0x10/0x20 > >> [8.318733] [] > >> kirin_fbdev_output_poll_changed+0x20/0x58 > >> [8.318740] [] > >> drm_kms_helper_hotplug_event+0x28/0x38 > >> [8.318748] [] drm_helper_hpd_irq_event+0x138/0x180 > >> [8.318754] [] adv7511_irq_process+0x78/0xd8 > >> [8.318761] [] adv7511_irq_handler+0x14/0x28 > >> [8.318769] [] irq_thread_fn+0x28/0x68 > >> [8.318775] [] irq_thread+0x128/0x1e8 > >> [8.318782] [] kthread+0xd0/0xe8 > >> [8.318788] [] ret_from_fork+0x10/0x50 > >> > >> So we're actually in irq handling the hotplug interrupt, which is why > >> we never get the irq notification when the edid is read. > >> > >> I suspect we need to use a workqueue to do the hotplug handling out of > >> irq. > > > > Lovely :-) > > > > Quoting the DRM documentation: > > > > /** > > * drm_helper_hpd_irq_event - hotplug processing > > * @dev: drm_device > > * > > * Drivers can use this helper function to run a detect cycle on all > > connectors > > * which have the DRM_CONNECTOR_POLL_HPD flag set in their &polled member. > > All > > * other connectors are ignored, which is useful to avoid reprobing fixed > > * panels. > > * > > * This helper function is useful for drivers which can't or don't track > > hotplug > > * interrupts for each connector. > > * > > * Drivers which support hotplug interrupts for each connector > > individually and > > * which have a more fine-grained detect logic should bypass this code and > > * directly call drm_kms_helper_hotplug_event() in case the connector > > state > > * changed. > > * > > * This function must be called from process context with no mode > > * setting locks held. > > * > > * Note that a connector can be both polled and probed from the hotplug > > handler, > > * in case the hotplug interrupt is known to be unreliable. > > */ > > > > So it looks like we should use drm_kms_helper_hotplug_event() instead. > > > > /** > > * drm_kms_helper_hotplug_event - fire off KMS hotplug events > > * @dev: drm_device whose connector state changed > > * > > * This function fires off the uevent for userspace and also calls the > > * output_poll_changed function, which is most commonly used to inform the > > fbdev > > * emulation code and allow it to update the fbcon output configuration. > > * > > * Drivers should call this from their hotplug handling code when a change > > is > > * detected. Note that this function does not do any output detection of > > its > > * own, like drm_helper_hpd_irq_event() does - this is assumed to be done > > by the > > * driver already. > > * > > * This function must be called from process context with no mode > > * setting locks held. > > */ > > > > The function suffers from the same problem though, that it must be called > > from process context. > > > > Daniel, why do we have an API the is clearly related to interrupt handling > > but requires the caller to implement a workqueue ? > > Because in general you need that workqueue anyway, and up to now there was > no driver ever who didn't have a work-queue already. None of the bridge drivers in drivers/gpu/drm/bridge/ have workqueues. They call the HPD helpers from a threaded interrupt handler though. Sleeping in that context is fine, calling functions that might rely on interrupts from the same device to signal completion (such as reading EDID through .get_modes()) isn't. > Nesting workqueues > within workqueues seemed beyond silly, hence why I removed them in: > > commit 69787f7da6b2adc4054357a661aaa1701a9ca76f > Author: Daniel Vetter > Date: Tue Oct 23 18:23:34 2012 + > > drm: run the hpd irq event code directly > > I guess we could talk about re-introducing a work-item based version of > drm_helper_hpd_irq_event.
[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
On Tue, Nov 22, 2016 at 08:23:38PM +0200, Laurent Pinchart wrote: > Hi John, > > (CC'ing Daniel) > > On Tuesday 22 Nov 2016 10:07:53 John Stultz wrote: > > On Tue, Nov 22, 2016 at 9:38 AM, John Stultz > > wrote: > > > Interestingly, without the msleep added in this patch, removing the > > > wait_event_interruptible_timeout() method in adv7511_wait_for_edid() > > > and using the polling loop seems to make things just as reliable. So > > > maybe something is off with the irq handling here instead? > > > > A.. So I think the trouble here is the that when we fail waiting > > for the irq, the backtrace is as follows: > > > > [8.318654] [] dump_backtrace+0x0/0x1a0 > > [8.318661] [] show_stack+0x14/0x20 > > [8.318671] [] dump_stack+0x90/0xb0 > > [8.318680] [] adv7511_get_edid_block+0x2c8/0x320 > > [8.318687] [] drm_do_get_edid+0x78/0x280 > > [8.318693] [] adv7511_get_modes+0x80/0xd8 > > [8.318700] [] adv7511_connector_get_modes+0x14/0x20 > > [8.318710] [] > > drm_helper_probe_single_connector_modes+0x2bc/0x500 > > [8.318718] [] drm_fb_helper_hotplug_event+0x130/0x188 > > [8.318726] [] drm_fbdev_cma_hotplug_event+0x10/0x20 > > [8.318733] [] > > kirin_fbdev_output_poll_changed+0x20/0x58 > > [8.318740] [] drm_kms_helper_hotplug_event+0x28/0x38 > > [8.318748] [] drm_helper_hpd_irq_event+0x138/0x180 > > [8.318754] [] adv7511_irq_process+0x78/0xd8 > > [8.318761] [] adv7511_irq_handler+0x14/0x28 > > [8.318769] [] irq_thread_fn+0x28/0x68 > > [8.318775] [] irq_thread+0x128/0x1e8 > > [8.318782] [] kthread+0xd0/0xe8 > > [8.318788] [] ret_from_fork+0x10/0x50 > > > > So we're actually in irq handling the hotplug interrupt, which is why > > we never get the irq notification when the edid is read. > > > > I suspect we need to use a workqueue to do the hotplug handling out of irq. > > Lovely :-) > > Quoting the DRM documentation: > > /** > * drm_helper_hpd_irq_event - hotplug processing > * @dev: drm_device > * > * Drivers can use this helper function to run a detect cycle on all > connectors > * which have the DRM_CONNECTOR_POLL_HPD flag set in their &polled member. All > * other connectors are ignored, which is useful to avoid reprobing fixed > * panels. > * > * This helper function is useful for drivers which can't or don't track > hotplug > * interrupts for each connector. > * > * Drivers which support hotplug interrupts for each connector individually > and > * which have a more fine-grained detect logic should bypass this code and > * directly call drm_kms_helper_hotplug_event() in case the connector state > * changed. > * > * This function must be called from process context with no mode > * setting locks held. > * > * Note that a connector can be both polled and probed from the hotplug > handler, > * in case the hotplug interrupt is known to be unreliable. > */ > > So it looks like we should use drm_kms_helper_hotplug_event() instead. > > /** > * drm_kms_helper_hotplug_event - fire off KMS hotplug events > * @dev: drm_device whose connector state changed > * > * This function fires off the uevent for userspace and also calls the > * output_poll_changed function, which is most commonly used to inform the > fbdev > * emulation code and allow it to update the fbcon output configuration. > * > * Drivers should call this from their hotplug handling code when a change is > * detected. Note that this function does not do any output detection of its > * own, like drm_helper_hpd_irq_event() does - this is assumed to be done by > the > * driver already. > * > * This function must be called from process context with no mode > * setting locks held. > */ > > The function suffers from the same problem though, that it must be called > from > process context. > > Daniel, why do we have an API the is clearly related to interrupt handling > but > requires the caller to implement a workqueue ? Because in general you need that workqueue anyway, and up to now there was no driver ever who didn't have a work-queue already. Nesting workqueues within workqueues seemed beyond silly, hence why I removed them in: commit 69787f7da6b2adc4054357a661aaa1701a9ca76f Author: Daniel Vetter Date: Tue Oct 23 18:23:34 2012 + drm: run the hpd irq event code directly I guess we could talk about re-introducing a work-item based version of drm_helper_hpd_irq_event. But for drm_kms_helper_hotplug_event I think it doesn't make sense - if you call that you've probably just done a pile of i2c transactions, and those can sleep. If you haven't done i2c transactions, then it's not an external panel, and why exactly are you handling hpd for them? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
Hi John, (CC'ing Daniel) On Tuesday 22 Nov 2016 10:07:53 John Stultz wrote: > On Tue, Nov 22, 2016 at 9:38 AM, John Stultz > wrote: > > Interestingly, without the msleep added in this patch, removing the > > wait_event_interruptible_timeout() method in adv7511_wait_for_edid() > > and using the polling loop seems to make things just as reliable. So > > maybe something is off with the irq handling here instead? > > A.. So I think the trouble here is the that when we fail waiting > for the irq, the backtrace is as follows: > > [8.318654] [] dump_backtrace+0x0/0x1a0 > [8.318661] [] show_stack+0x14/0x20 > [8.318671] [] dump_stack+0x90/0xb0 > [8.318680] [] adv7511_get_edid_block+0x2c8/0x320 > [8.318687] [] drm_do_get_edid+0x78/0x280 > [8.318693] [] adv7511_get_modes+0x80/0xd8 > [8.318700] [] adv7511_connector_get_modes+0x14/0x20 > [8.318710] [] > drm_helper_probe_single_connector_modes+0x2bc/0x500 > [8.318718] [] drm_fb_helper_hotplug_event+0x130/0x188 > [8.318726] [] drm_fbdev_cma_hotplug_event+0x10/0x20 > [8.318733] [] > kirin_fbdev_output_poll_changed+0x20/0x58 > [8.318740] [] drm_kms_helper_hotplug_event+0x28/0x38 > [8.318748] [] drm_helper_hpd_irq_event+0x138/0x180 > [8.318754] [] adv7511_irq_process+0x78/0xd8 > [8.318761] [] adv7511_irq_handler+0x14/0x28 > [8.318769] [] irq_thread_fn+0x28/0x68 > [8.318775] [] irq_thread+0x128/0x1e8 > [8.318782] [] kthread+0xd0/0xe8 > [8.318788] [] ret_from_fork+0x10/0x50 > > So we're actually in irq handling the hotplug interrupt, which is why > we never get the irq notification when the edid is read. > > I suspect we need to use a workqueue to do the hotplug handling out of irq. Lovely :-) Quoting the DRM documentation: /** * drm_helper_hpd_irq_event - hotplug processing * @dev: drm_device * * Drivers can use this helper function to run a detect cycle on all connectors * which have the DRM_CONNECTOR_POLL_HPD flag set in their &polled member. All * other connectors are ignored, which is useful to avoid reprobing fixed * panels. * * This helper function is useful for drivers which can't or don't track hotplug * interrupts for each connector. * * Drivers which support hotplug interrupts for each connector individually and * which have a more fine-grained detect logic should bypass this code and * directly call drm_kms_helper_hotplug_event() in case the connector state * changed. * * This function must be called from process context with no mode * setting locks held. * * Note that a connector can be both polled and probed from the hotplug handler, * in case the hotplug interrupt is known to be unreliable. */ So it looks like we should use drm_kms_helper_hotplug_event() instead. /** * drm_kms_helper_hotplug_event - fire off KMS hotplug events * @dev: drm_device whose connector state changed * * This function fires off the uevent for userspace and also calls the * output_poll_changed function, which is most commonly used to inform the fbdev * emulation code and allow it to update the fbcon output configuration. * * Drivers should call this from their hotplug handling code when a change is * detected. Note that this function does not do any output detection of its * own, like drm_helper_hpd_irq_event() does - this is assumed to be done by the * driver already. * * This function must be called from process context with no mode * setting locks held. */ The function suffers from the same problem though, that it must be called from process context. Daniel, why do we have an API the is clearly related to interrupt handling but requires the caller to implement a workqueue ? -- Regards, Laurent Pinchart
[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
On Tue, Nov 22, 2016 at 10:23 AM, Laurent Pinchart wrote: > On Tuesday 22 Nov 2016 10:07:53 John Stultz wrote: >> On Tue, Nov 22, 2016 at 9:38 AM, John Stultz >> wrote: >> > Interestingly, without the msleep added in this patch, removing the >> > wait_event_interruptible_timeout() method in adv7511_wait_for_edid() >> > and using the polling loop seems to make things just as reliable. So >> > maybe something is off with the irq handling here instead? >> >> A.. So I think the trouble here is the that when we fail waiting >> for the irq, the backtrace is as follows: >> >> [8.318654] [] dump_backtrace+0x0/0x1a0 >> [8.318661] [] show_stack+0x14/0x20 >> [8.318671] [] dump_stack+0x90/0xb0 >> [8.318680] [] adv7511_get_edid_block+0x2c8/0x320 >> [8.318687] [] drm_do_get_edid+0x78/0x280 >> [8.318693] [] adv7511_get_modes+0x80/0xd8 >> [8.318700] [] adv7511_connector_get_modes+0x14/0x20 >> [8.318710] [] >> drm_helper_probe_single_connector_modes+0x2bc/0x500 >> [8.318718] [] drm_fb_helper_hotplug_event+0x130/0x188 >> [8.318726] [] drm_fbdev_cma_hotplug_event+0x10/0x20 >> [8.318733] [] >> kirin_fbdev_output_poll_changed+0x20/0x58 >> [8.318740] [] drm_kms_helper_hotplug_event+0x28/0x38 >> [8.318748] [] drm_helper_hpd_irq_event+0x138/0x180 >> [8.318754] [] adv7511_irq_process+0x78/0xd8 >> [8.318761] [] adv7511_irq_handler+0x14/0x28 >> [8.318769] [] irq_thread_fn+0x28/0x68 >> [8.318775] [] irq_thread+0x128/0x1e8 >> [8.318782] [] kthread+0xd0/0xe8 >> [8.318788] [] ret_from_fork+0x10/0x50 >> >> So we're actually in irq handling the hotplug interrupt, which is why >> we never get the irq notification when the edid is read. >> >> I suspect we need to use a workqueue to do the hotplug handling out of irq. So yea, using schedule_work on a work_struct to defer the hotplug handling seems to solve this issue. Thanks again for pushing back on the msleep approach. :) > Lovely :-) > > Quoting the DRM documentation: > > /** > * drm_helper_hpd_irq_event - hotplug processing > * @dev: drm_device > * > * Drivers can use this helper function to run a detect cycle on all > connectors > * which have the DRM_CONNECTOR_POLL_HPD flag set in their &polled member. All > * other connectors are ignored, which is useful to avoid reprobing fixed > * panels. > * > * This helper function is useful for drivers which can't or don't track > hotplug > * interrupts for each connector. > * > * Drivers which support hotplug interrupts for each connector individually > and > * which have a more fine-grained detect logic should bypass this code and > * directly call drm_kms_helper_hotplug_event() in case the connector state > * changed. > * > * This function must be called from process context with no mode > * setting locks held. > * > * Note that a connector can be both polled and probed from the hotplug > handler, > * in case the hotplug interrupt is known to be unreliable. > */ > > So it looks like we should use drm_kms_helper_hotplug_event() instead. Ok. I'll switch to this in my patch set as well. It doesn't seem to have any behavioral effect that I can see right off. thanks -john
[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
Hi John, Thank you for the patch. On Monday 21 Nov 2016 16:37:31 John Stultz wrote: > Secton 4.1 of the adv7511 programming guide advises one waits > 200ms after powering on the chip before trying to communicate > with it via i2c. Not doing so can cause reliability issues when > probing the EDID. > > See: > http://www.analog.com/media/en/technical-documentation/user-guides/ADV7511_P > rogramming_Guide.pdf > > So this patch simply adds a 200ms sleep at the end of the > power_on path. This greatly improves EDID probing reliabilty > on hotplug with the HiKey device. > > Cc: David Airlie > Cc: Archit Taneja > Cc: Wolfram Sang > Cc: Lars-Peter Clausen > Cc: Laurent Pinchart > Cc: dri-devel at lists.freedesktop.org > Signed-off-by: John Stultz > --- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index b240e05..2114a4c > 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -361,6 +361,8 @@ static void __adv7511_power_on(struct adv7511 *adv7511) >*/ > regcache_sync(adv7511->regmap); > > + msleep(200); > + The documentation states that "The user should wait 200ms for the address to be decided, after the power supplies are high, before attempting to communicate with the ADV7511W using I2C." The hardware user's guide further states that "When initially powered up, there is a 200ms period before the device is ready to be addressed." Not only the delay you add comes after lots of I2C communication, but the driver doesn't handle regulators, and thus doesn't power down the device at the hardware level. The initial power up should thus be long gone when this code is reached. Could it be that, on the HiKey board, the power supply is controlled through another mean that doesn't comply with the 200ms rule ? > if (adv7511->type == ADV7533) > adv7533_dsi_power_on(adv7511); > } -- Regards, Laurent Pinchart
[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
On Tue, Nov 22, 2016 at 9:38 AM, John Stultz wrote: > > Interestingly, without the msleep added in this patch, removing the > wait_event_interruptible_timeout() method in adv7511_wait_for_edid() > and using the polling loop seems to make things just as reliable. So > maybe something is off with the irq handling here instead? A.. So I think the trouble here is the that when we fail waiting for the irq, the backtrace is as follows: [8.318654] [] dump_backtrace+0x0/0x1a0 [8.318661] [] show_stack+0x14/0x20 [8.318671] [] dump_stack+0x90/0xb0 [8.318680] [] adv7511_get_edid_block+0x2c8/0x320 [8.318687] [] drm_do_get_edid+0x78/0x280 [8.318693] [] adv7511_get_modes+0x80/0xd8 [8.318700] [] adv7511_connector_get_modes+0x14/0x20 [8.318710] [] drm_helper_probe_single_connector_modes+0x2bc/0x500 [8.318718] [] drm_fb_helper_hotplug_event+0x130/0x188 [8.318726] [] drm_fbdev_cma_hotplug_event+0x10/0x20 [8.318733] [] kirin_fbdev_output_poll_changed+0x20/0x58 [8.318740] [] drm_kms_helper_hotplug_event+0x28/0x38 [8.318748] [] drm_helper_hpd_irq_event+0x138/0x180 [8.318754] [] adv7511_irq_process+0x78/0xd8 [8.318761] [] adv7511_irq_handler+0x14/0x28 [8.318769] [] irq_thread_fn+0x28/0x68 [8.318775] [] irq_thread+0x128/0x1e8 [8.318782] [] kthread+0xd0/0xe8 [8.318788] [] ret_from_fork+0x10/0x50 So we're actually in irq handling the hotplug interrupt, which is why we never get the irq notification when the edid is read. I suspect we need to use a workqueue to do the hotplug handling out of irq. thanks -john
[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
On Tue, Nov 22, 2016 at 12:25 AM, Laurent Pinchart
wrote:
> On Monday 21 Nov 2016 16:37:31 John Stultz wrote:
>> Secton 4.1 of the adv7511 programming guide advises one waits
>> 200ms after powering on the chip before trying to communicate
>> with it via i2c. Not doing so can cause reliability issues when
>> probing the EDID.
>>
>> See:
>> http://www.analog.com/media/en/technical-documentation/user-guides/ADV7511_P
>> rogramming_Guide.pdf
>>
>> So this patch simply adds a 200ms sleep at the end of the
>> power_on path. This greatly improves EDID probing reliabilty
>> on hotplug with the HiKey device.
>>
[snip]
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -361,6 +361,8 @@ static void __adv7511_power_on(struct adv7511 *adv7511)
>>*/
>> regcache_sync(adv7511->regmap);
>>
>> + msleep(200);
>> +
>
> The documentation states that
>
> "The user should wait 200ms for the address to be decided, after the power
> supplies are high, before attempting to communicate with the ADV7511W using
> I2C."
>
> The hardware user's guide further states that
>
> "When initially powered up, there is a 200ms period before the device is ready
> to be addressed."
>
> Not only the delay you add comes after lots of I2C communication, but the
> driver doesn't handle regulators, and thus doesn't power down the device at
> the hardware level. The initial power up should thus be long gone when this
> code is reached.
>
> Could it be that, on the HiKey board, the power supply is controlled through
> another mean that doesn't comply with the 200ms rule ?
Thanks so much for the clarifications. Hopefully my confusion around
flipping the POWER_DOWN register and the actual power to the chip is
understandable. :)
Hrm... So apparently I was mixing the result of this change up with
the refactoring of the power_on logic in the previous patch.
So initially I found this as I was seeing i2c_transfer() frequently
return errors when trying to read the EDID, after turning off the
ADV7511_POWER_POWER_DOWN register. The explanation in the guide to
wait 200ms made sense, and seemed to make the i2c_transfer errors go
away. But it ends up the i2c_tranfer errors are fixed by re-using the
power_on logic in the patch before.
However this patch still improves things, as without it I'm seeing the
DDCController status in adv7511_get_edid_block() being 1 ("reading
edid") and the adv7511_wait_for_edid() regularly times out. It seems
like we don't get the irq and set the edid_read bit we're waiting on
until much later (after we decide there's no edid and try to start
using with 800x600).
Interestingly, without the msleep added in this patch, removing the
wait_event_interruptible_timeout() method in adv7511_wait_for_edid()
and using the polling loop seems to make things just as reliable. So
maybe something is off with the irq handling here instead?
thanks
-john
[RFC][PATCH 2/3] drm/bridge: adv7511: Add 200ms delay on power-on
Secton 4.1 of the adv7511 programming guide advises one waits 200ms after powering on the chip before trying to communicate with it via i2c. Not doing so can cause reliability issues when probing the EDID. See: http://www.analog.com/media/en/technical-documentation/user-guides/ADV7511_Programming_Guide.pdf So this patch simply adds a 200ms sleep at the end of the power_on path. This greatly improves EDID probing reliabilty on hotplug with the HiKey device. Cc: David Airlie Cc: Archit Taneja Cc: Wolfram Sang Cc: Lars-Peter Clausen Cc: Laurent Pinchart Cc: dri-devel at lists.freedesktop.org Signed-off-by: John Stultz --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index b240e05..2114a4c 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -361,6 +361,8 @@ static void __adv7511_power_on(struct adv7511 *adv7511) */ regcache_sync(adv7511->regmap); + msleep(200); + if (adv7511->type == ADV7533) adv7533_dsi_power_on(adv7511); } -- 2.7.4
