From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 17/19] drm/i915: vlv: factor out valleyview_display_irq_install Date: Mon, 24 Feb 2014 15:34:39 +0200 Message-ID: <1393248879.13131.93.camel@intelbox> References: <1392674540-10915-1-git-send-email-imre.deak@intel.com> <1392674540-10915-18-git-send-email-imre.deak@intel.com> <20140220115616.023016a5@jbarnes-desktop> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1712630232==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 62CB9FA1D5 for ; Mon, 24 Feb 2014 05:35:19 -0800 (PST) In-Reply-To: <20140220115616.023016a5@jbarnes-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1712630232== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-w9MuhxBMfpMamw3Rd6uk" --=-w9MuhxBMfpMamw3Rd6uk Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2014-02-20 at 11:56 -0800, Jesse Barnes wrote: > On Tue, 18 Feb 2014 00:02:18 +0200 > Imre Deak wrote: >=20 > > We'll need to disable/re-enable the display-side IRQs when turning > > off/on the VLV display power well. Factor out the helper functions > > for this. For now keep the display IRQs enabled by default, so the > > functionality doesn't change. This will be changed to enable/disable > > the IRQs on-demand when adding support for VLV power wells in an > > upcoming patch. > >=20 > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/i915_dma.c | 1 + > > drivers/gpu/drm/i915/i915_drv.h | 5 ++ > > drivers/gpu/drm/i915/i915_irq.c | 122 ++++++++++++++++++++++++++++++++= -------- > > 3 files changed, 103 insertions(+), 25 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i91= 5_dma.c > > index f8f7a59..dca4dc3 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -1668,6 +1668,7 @@ int i915_driver_load(struct drm_device *dev, unsi= gned long flags) > > goto out_mtrrfree; > > } > > =20 > > + dev_priv->display_irqs_enabled =3D true; > > intel_irq_init(dev); > > intel_uncore_sanitize(dev); > > =20 > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i91= 5_drv.h > > index 632f9d8..227c349 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1454,6 +1454,8 @@ typedef struct drm_i915_private { > > /* protects the irq masks */ > > spinlock_t irq_lock; > > =20 > > + bool display_irqs_enabled; > > + > > /* To control wakeup latency, e.g. for irq-driven dp aux transfers. *= / > > struct pm_qos_request pm_qos; > > =20 > > @@ -2052,6 +2054,9 @@ void > > i915_disable_pipestat(drm_i915_private_t *dev_priv, enum pipe pipe, > > u32 status_mask); > > =20 > > +void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)= ; > > +void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv= ); > > + > > /* i915_gem.c */ > > int i915_gem_init_ioctl(struct drm_device *dev, void *data, > > struct drm_file *file_priv); > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i91= 5_irq.c > > index 75dd0a8..6078d06 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -3016,41 +3016,109 @@ static int ironlake_irq_postinstall(struct drm= _device *dev) > > return 0; > > } > > =20 > > +static void valleyview_display_irqs_install(struct drm_i915_private *d= ev_priv) > > +{ > > + unsigned long irqflags; > > + u32 pipestat_mask; > > + u32 iir_mask; > > + > > + pipestat_mask =3D PIPESTAT_INT_STATUS_MASK | > > + PIPE_FIFO_UNDERRUN_STATUS; > > + I915_WRITE(PIPESTAT(PIPE_A), pipestat_mask); > > + I915_WRITE(PIPESTAT(PIPE_B), pipestat_mask); > > + POSTING_READ(PIPESTAT(PIPE_A)); > > + > > + pipestat_mask =3D PLANE_FLIP_DONE_INT_STATUS_VLV | > > + PIPE_CRC_DONE_INTERRUPT_STATUS; > > + > > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > > + i915_enable_pipestat(dev_priv, PIPE_A, pipestat_mask | > > + PIPE_GMBUS_INTERRUPT_STATUS); > > + i915_enable_pipestat(dev_priv, PIPE_B, pipestat_mask); > > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > > + > > + iir_mask =3D I915_DISPLAY_PORT_INTERRUPT | > > + I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | > > + I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; > > + dev_priv->irq_mask &=3D ~iir_mask; > > + > > + I915_WRITE(VLV_IIR, iir_mask); > > + I915_WRITE(VLV_IIR, iir_mask); > > + I915_WRITE(VLV_IMR, dev_priv->irq_mask); > > + I915_WRITE(VLV_IER, ~dev_priv->irq_mask); > > + POSTING_READ(VLV_IER); > > +} > > + > > +static void valleyview_display_irqs_uninstall(struct drm_i915_private = *dev_priv) > > +{ > > + unsigned long irqflags; > > + u32 pipestat_mask; > > + u32 iir_mask; > > + > > + iir_mask =3D I915_DISPLAY_PORT_INTERRUPT | > > + I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | > > + I915_DISPLAY_PIPE_A_EVENT_INTERRUPT; > > + dev_priv->irq_mask |=3D iir_mask; > > + I915_WRITE(VLV_IER, ~dev_priv->irq_mask); > > + I915_WRITE(VLV_IMR, dev_priv->irq_mask); > > + I915_WRITE(VLV_IIR, iir_mask); > > + I915_WRITE(VLV_IIR, iir_mask); > > + POSTING_READ(VLV_IIR); > > + > > + pipestat_mask =3D PLANE_FLIP_DONE_INT_STATUS_VLV | > > + PIPE_CRC_DONE_INTERRUPT_STATUS; > > + > > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > > + i915_disable_pipestat(dev_priv, PIPE_A, pipestat_mask | > > + PIPE_GMBUS_INTERRUPT_STATUS); > > + i915_disable_pipestat(dev_priv, PIPE_B, pipestat_mask); > > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > > + > > + pipestat_mask =3D PIPESTAT_INT_STATUS_MASK | > > + PIPE_FIFO_UNDERRUN_STATUS; > > + I915_WRITE(PIPESTAT(PIPE_A), pipestat_mask); > > + I915_WRITE(PIPESTAT(PIPE_B), pipestat_mask); > > + POSTING_READ(PIPESTAT(PIPE_A)); > > +} > > + > > +void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv) > > +{ > > + if (dev_priv->display_irqs_enabled) > > + return; > > + > > + dev_priv->display_irqs_enabled =3D true; > > + > > + if (dev_priv->dev->irq_enabled) > > + valleyview_display_irqs_install(dev_priv); > > +} > > + > > +void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv= ) > > +{ > > + if (!dev_priv->display_irqs_enabled) > > + return; > > + > > + dev_priv->display_irqs_enabled =3D false; > > + > > + if (dev_priv->dev->irq_enabled) > > + valleyview_display_irqs_uninstall(dev_priv); > > +} > > + > > static int valleyview_irq_postinstall(struct drm_device *dev) > > { > > drm_i915_private_t *dev_priv =3D (drm_i915_private_t *) dev->dev_priv= ate; > > - u32 enable_mask; > > - u32 pipestat_enable =3D PLANE_FLIP_DONE_INT_STATUS_VLV | > > - PIPE_CRC_DONE_INTERRUPT_STATUS; > > - unsigned long irqflags; > > =20 > > - enable_mask =3D I915_DISPLAY_PORT_INTERRUPT; > > - enable_mask |=3D I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | > > - I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; > > - > > - /* > > - *Leave vblank interrupts masked initially. enable/disable will > > - * toggle them based on usage. > > - */ > > - dev_priv->irq_mask =3D ~enable_mask; > > + dev_priv->irq_mask =3D ~0; > > =20 > > I915_WRITE(PORT_HOTPLUG_EN, 0); > > POSTING_READ(PORT_HOTPLUG_EN); > > =20 > > I915_WRITE(VLV_IMR, dev_priv->irq_mask); > > - I915_WRITE(VLV_IER, enable_mask); > > + I915_WRITE(VLV_IER, ~dev_priv->irq_mask); > > I915_WRITE(VLV_IIR, 0xffffffff); > > - I915_WRITE(PIPESTAT(0), 0xffff); > > - I915_WRITE(PIPESTAT(1), 0xffff); > > POSTING_READ(VLV_IER); > > =20 > > - /* Interrupt setup is already guaranteed to be single-threaded, this = is > > - * just to make the assert_spin_locked check happy. */ > > - spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > > - i915_enable_pipestat(dev_priv, PIPE_A, pipestat_enable); > > - i915_enable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_INTERRUPT_STATUS); > > - i915_enable_pipestat(dev_priv, PIPE_B, pipestat_enable); > > - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > > + if (dev_priv->display_irqs_enabled) > > + valleyview_display_irqs_install(dev_priv); > > =20 > > I915_WRITE(VLV_IIR, 0xffffffff); > > I915_WRITE(VLV_IIR, 0xffffffff); > > @@ -3193,8 +3261,12 @@ static void valleyview_irq_uninstall(struct drm_= device *dev) > > I915_WRITE(HWSTAM, 0xffffffff); > > I915_WRITE(PORT_HOTPLUG_EN, 0); > > I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT)); > > - for_each_pipe(pipe) > > - I915_WRITE(PIPESTAT(pipe), 0xffff); > > + > > + if (dev_priv->display_irqs_enabled) > > + valleyview_display_irqs_uninstall(dev_priv); > > + > > + dev_priv->irq_mask =3D 0; > > + > > I915_WRITE(VLV_IIR, 0xffffffff); > > I915_WRITE(VLV_IMR, 0xffffffff); > > I915_WRITE(VLV_IER, 0x0); >=20 > IRQ enable state and reg state may be another good thing to cross > check. A good idea, I can add that check as a follow-up. > And on disable, it might be a good thing to just write all of > the pipestat irq status bits to avoid stuck interrupts (I think the > order is ok though). I added + pipestat_mask =3D PIPESTAT_INT_STATUS_MASK | + PIPE_FIFO_UNDERRUN_STATUS; + I915_WRITE(PIPESTAT(PIPE_A), pipestat_mask); + I915_WRITE(PIPESTAT(PIPE_B), pipestat_mask); which does that. > But you've probably tested it already so I assume it works ok. kms_flip passed except the modset_vs_hang subtests which have an open bugzilla ticket and I think are unrelated to this change. One more thing I noticed meanwhile, is that the locking in valleyview_display_irqs_install/uninstall() should be around all register accesses, since we can call these functions with interrupts already enabled. --Imre >=20 > Reviewed-by: Jesse Barnes >=20 --=-w9MuhxBMfpMamw3Rd6uk Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQEcBAABAgAGBQJTC0pvAAoJEORIIAnNuWDFuUYH/3xf4XT6CX0dPdNzrSIJ1311 FhUoQpA1cbc7eXj81zFdz9+t86othMJR0CZYkFLawaz90hxw+QNVTmJVk9NLCO0A VpzO0Tc8LuCfD5+thglw7Z1RbxG5AtlT16hiIlNfo+ETbWH1WNjRQmCtQq6aeBQe JRoJNwfdjdcsaXtb1LRZcW5bv7OE7LP0NAug9Mo2BLUbXsN8xK4ILQCOb/MR2WmQ POxCchqAiUsY4ESPukf3sWjPz+ax37D5HD3IAFsSuuvCHvHv0owSIGFnTxu53p/9 1GmIPYBNrbTXP6wki/Ga2MBXDa3eLecq3aIjAOajqkTLxM8Z8Sf/zjmBh9iakWI= =9Mq1 -----END PGP SIGNATURE----- --=-w9MuhxBMfpMamw3Rd6uk-- --===============1712630232== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1712630232==--