On Wed, Oct 07, 2020 at 06:15:47PM -0400, Lyude Paul wrote: > On Wed, 2020-10-07 at 22:22 +0300, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Currently we call .hpd_irq_setup() directly just before display > > resume, and follow it with another call via intel_hpd_init() > > just afterwards. Assuming the hpd pins are marked as enabled > > during the open-coded call these two things do exactly the > > same thing (ie. enable HPD interrupts). Which even makes sense > > since we definitely need working HPD interrupts for MST sideband > > during the display resume. > > > > So let's nuke the open-coded call and move the intel_hpd_init() > > call earlier. However we need to leave the poll_init_work stuff > > behind after the display resume as that will trigger display > > detection while we're resuming. We don't want that trampling over > > the display resume process. To make this a bit more symmetric > > we turn this into a intel_hpd_poll_{enable,disable}() pair. > > So we end up with the following transformation: > > intel_hpd_poll_init() -> intel_hpd_poll_enable() > > lone intel_hpd_init() -> intel_hpd_init()+intel_hpd_poll_disable() > > .hpd_irq_setup()+resume+intel_hpd_init() -> > > intel_hpd_init()+resume+intel_hpd_poll_disable() > > > > If we really would like to prevent all *long* HPD processing during > > display resume we'd need some kind of software mechanism to simply > > ignore all long HPDs. Currently we appear to have that just for > > fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and > > right all the time I guess that's mostly sufficient. > > > > For a bit of history on this, we first got a mechanism to block > > hotplug processing during suspend in commit 15239099d7a7 ("drm/i915: > > enable irqs earlier when resuming") on account of moving the irq enable > > earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper: > > Fix hpd vs. initial config races") because the fdev initial config > > got pushed to a later point. The second ad-hoc hpd_irq_setup() for > > resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST > > support (v0.7)") to be able to do MST sideband during the resume. > > And finally we got a partial resurrection of the hpd blocking > > mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD > > processing during suspend"), but this time it only prevent fbdev > > from handling hpd while resuming. > > > > v2: Leave the poll_init_work behind > > > > Cc: Lyude Paul <lyude@xxxxxxxxxx> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 9 ++-- > > .../drm/i915/display/intel_display_power.c | 3 +- > > drivers/gpu/drm/i915/display/intel_hotplug.c | 42 ++++++++++++++----- > > drivers/gpu/drm/i915/display/intel_hotplug.h | 3 +- > > drivers/gpu/drm/i915/i915_drv.c | 23 ++++------ > > 5 files changed, 46 insertions(+), 34 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 907e1d155443..0d5607ae97c4 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -5036,18 +5036,15 @@ void intel_finish_reset(struct drm_i915_private > > *dev_priv) > > intel_pps_unlock_regs_wa(dev_priv); > > intel_modeset_init_hw(dev_priv); > > intel_init_clock_gating(dev_priv); > > - > > - spin_lock_irq(&dev_priv->irq_lock); > > - if (dev_priv->display.hpd_irq_setup) > > - dev_priv->display.hpd_irq_setup(dev_priv); > > - spin_unlock_irq(&dev_priv->irq_lock); > > + intel_hpd_init(dev_priv); > > + intel_hpd_poll_disable(dev_priv); > > > > ret = __intel_display_resume(dev, state, ctx); > > if (ret) > > drm_err(&dev_priv->drm, > > "Restoring old state failed with %i\n", ret); > > > > - intel_hpd_init(dev_priv); > > + intel_hpd_poll_disable(dev_priv); > > Looks like you're calling intel_hpd_poll_disable() twice here, is this > intentional? No, just a failure to follow my own rules. Thanks for spotting it. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx