On Tue, Oct 06, 2020 at 09:58:07PM +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 just nuke the open-coded call and move the intel_hpd_init()
> call earlier.
>
> 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.
Daniel suggested including the archaeological record here.
This is what I was planning to amend here:
"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 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 the hpd while resuming."
But looks like I was far too optimistic and this did in fact blow
up in CI. So I'll need to do some actual work to figure out what's
going on...
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 8 +-------
> drivers/gpu/drm/i915/i915_drv.c | 16 ++--------------
> 2 files changed, 3 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 907e1d155443..87df7167433f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5036,18 +5036,12 @@ 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);
>
> 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);
> }
>
> drm_atomic_state_put(state);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ebc15066d108..b2a050d916e3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1226,26 +1226,14 @@ static int i915_drm_resume(struct drm_device *dev)
>
> intel_modeset_init_hw(dev_priv);
> intel_init_clock_gating(dev_priv);
> + intel_hpd_init(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);
> -
> + /* MST sideband requires HPD interrupts enabled */
> intel_dp_mst_resume(dev_priv);
> -
> intel_display_resume(dev);
>
> drm_kms_helper_poll_enable(dev);
>
> - /*
> - * ... but also need to make sure that hotplug processing
> - * doesn't cause havoc. Like in the driver load code we don't
> - * bother with the tiny race here where we might lose hotplug
> - * notifications.
> - * */
> - intel_hpd_init(dev_priv);
> -
> intel_opregion_resume(dev_priv);
>
> intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
> --
> 2.26.2
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
