From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/4] drm/i915: cancel hotplug and dig_port work during suspend and unload Date: Tue, 12 Aug 2014 15:13:56 +0300 Message-ID: <20140812121356.GW4193@intel.com> References: <1407783257-29468-1-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 1F4FD6E080 for ; Tue, 12 Aug 2014 05:14:00 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1407783257-29468-1-git-send-email-imre.deak@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Imre Deak Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Aug 11, 2014 at 09:54:15PM +0300, Imre Deak wrote: > Make sure these work handlers don't run after we system suspend or > unload the driver. Note that we don't cancel the handlers during runtime > suspend. That could lead to a lockup, since we take a runtime PM ref > from the handlers themselves. Fortunaltely canceling there is not needed > since the RPM ref itself provides for the needed serialization. > = > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 3 +-- > 3 files changed, 10 insertions(+), 2 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_= drv.c > index ec96f9a..0653761 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -494,6 +494,13 @@ bool i915_semaphore_is_enabled(struct drm_device *de= v) > return true; > } > = > +void intel_hpd_cancel_work(struct drm_i915_private *dev_priv) > +{ > + cancel_work_sync(&dev_priv->hotplug_work); > + cancel_work_sync(&dev_priv->dig_port_work); Since dig_port_work can queue a hotplug_work shouldn't these two be swapped? I wonder if we should also clear hpd_event_bits and {long,short}_hpd_port_mask before cancelling the works? At least it might make the works end a bit quicker if the are already running. I also noticed that we don't seem to grab any rpm/powerwell references in ->hpd_pulse() or i915_digport_work_func(). That doesn't seem right. Or maybe you already addressed that in another patch? > + cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work); > +} > + > static int i915_drm_freeze(struct drm_device *dev) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > @@ -538,6 +545,7 @@ static int i915_drm_freeze(struct drm_device *dev) > flush_delayed_work(&dev_priv->rps.delayed_resume_work); > = > intel_runtime_pm_disable_interrupts(dev); > + intel_hpd_cancel_work(dev_priv); > = > intel_suspend_gt_powersave(dev); > = > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index 35d150a..8776847 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2218,6 +2218,7 @@ extern void i915_update_gfx_val(struct drm_i915_pri= vate *dev_priv); > int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on); > = > extern void intel_console_resume(struct work_struct *work); > +void intel_hpd_cancel_work(struct drm_i915_private *dev_priv); > = > /* i915_irq.c */ > void i915_queue_hangcheck(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index b3a3168..ab90301 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13100,8 +13100,7 @@ void intel_modeset_cleanup(struct drm_device *dev) > * experience fancy races otherwise. > */ > drm_irq_uninstall(dev); > - cancel_work_sync(&dev_priv->hotplug_work); > - cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work); > + intel_hpd_cancel_work(dev_priv); > dev_priv->pm._irqs_disabled =3D true; > = > /* > -- = > 1.8.4 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC