From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 7/7] drm/i915: vlv: handle only enabled pipestat interrupt events Date: Wed, 05 Feb 2014 17:22:26 +0200 Message-ID: <1391613746.30971.45.camel@intelbox> References: <1391542551-20239-1-git-send-email-imre.deak@intel.com> <1391542551-20239-8-git-send-email-imre.deak@intel.com> <20140205151145.GL3891@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0328997862==" Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id 0206940DE for ; Wed, 5 Feb 2014 07:22:29 -0800 (PST) In-Reply-To: <20140205151145.GL3891@intel.com> 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: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0328997862== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-iFD2J3utLXeE53oYsjBF" --=-iFD2J3utLXeE53oYsjBF Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2014-02-05 at 17:11 +0200, Ville Syrj=C3=A4l=C3=A4 wrote: > On Tue, Feb 04, 2014 at 09:35:51PM +0200, Imre Deak wrote: > > Atm we call the handlers for pending pipestat interrupt events even if > > they aren't explicitly enabled by i915_enable_pipestat(). This isn't an > > issue for events other than the vblank start event, since those are > > always enabled anyways. Otoh, we enable the vblank start event > > on-demand, so we'll end up calling the vblank handler at times when the= y > > are disabled. > >=20 > > I haven't checked if this causes any real problem, but for consistency > > and to remove some overhead we should still fix this by clearing / > > handling only the enabled interrupt events. Also this is a dependency > > for the upcoming VLV power domain patchset where we need to disable all > > the pipestat interrupts whenever the display power well is off. > >=20 > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/i915_irq.c | 33 ++++++++++++++++++++++++++++++--- > > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > > 3 files changed, 35 insertions(+), 3 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i91= 5_drv.h > > index 43f37ca..faca5b4 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1427,6 +1427,7 @@ typedef struct drm_i915_private { > > }; > > u32 gt_irq_mask; > > u32 pm_irq_mask; > > + u32 pipestat_irq_mask[I915_MAX_PIPES]; > > =20 > > struct work_struct hotplug_work; > > bool enable_hotplug_processing; > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i91= 5_irq.c > > index eea5398..2cac477 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -419,6 +419,16 @@ done: > > return ret; > > } > > =20 > > +static bool __cpu_fifo_underrun_reporting_enabled(struct drm_device *d= ev, > > + enum pipe pipe) > > +{ > > + struct drm_i915_private *dev_priv =3D dev->dev_private; > > + struct drm_crtc *crtc =3D dev_priv->pipe_to_crtc_mapping[pipe]; > > + struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > > + > > + return !intel_crtc->cpu_fifo_underrun_disabled; > > +} > > + > > /** > > * intel_set_pch_fifo_underrun_reporting - enable/disable FIFO underru= n messages > > * @dev: drm device > > @@ -520,6 +530,8 @@ i915_enable_pipestat(drm_i915_private_t *dev_priv, = enum pipe pipe, > > if ((pipestat & enable_mask) =3D=3D enable_mask) > > return; > > =20 > > + dev_priv->pipestat_irq_mask[pipe] |=3D status_mask; > > + > > /* Enable the interrupt, clear any pending status */ > > pipestat |=3D enable_mask | status_mask; > > I915_WRITE(reg, pipestat); > > @@ -550,6 +562,8 @@ i915_disable_pipestat(drm_i915_private_t *dev_priv,= enum pipe pipe, > > if ((pipestat & enable_mask) =3D=3D 0) > > return; > > =20 > > + dev_priv->pipestat_irq_mask[pipe] &=3D ~status_mask; > > + > > pipestat &=3D ~enable_mask; > > I915_WRITE(reg, pipestat); > > POSTING_READ(reg); > > @@ -1530,18 +1544,31 @@ static void gen6_rps_irq_handler(struct drm_i91= 5_private *dev_priv, u32 pm_iir) > > static void valleyview_pipestat_irq_handler(struct drm_device *dev, u3= 2 iir) > > { > > drm_i915_private_t *dev_priv =3D dev->dev_private; > > - u32 pipe_stats[I915_MAX_PIPES]; > > + u32 pipe_stats[I915_MAX_PIPES] =3D { }; > > int pipe; > > =20 > > spin_lock(&dev_priv->irq_lock); > > for_each_pipe(pipe) { > > - int reg =3D PIPESTAT(pipe); > > + int reg; > > + u32 mask; > > + > > + if (!dev_priv->pipestat_irq_mask[pipe] && > > + !__cpu_fifo_underrun_reporting_enabled(dev, pipe)) > > + continue; > > + > > + reg =3D PIPESTAT(pipe); > > pipe_stats[pipe] =3D I915_READ(reg); > > =20 > > /* > > * Clear the PIPE*STAT regs before the IIR > > */ > > - if (pipe_stats[pipe] & 0x8000ffff) > > + mask =3D PIPESTAT_INT_ENABLE_MASK | PIPE_FIFO_UNDERRUN_STATUS; >=20 > Maybe we should add PIPE_FIFO_UNDERRUN_STATUS to the mask only when the > underrun reporting is enabled. If someone goes and enables underrun > reporting just after valleyview_pipestat_irq_handler(), we'd > potentially report a stale underrun. Ok, will fix it. --Imre >=20 > You get to blame me for that one though, since the bug is already there > in the current code, which I implemented. >=20 > > + if (iir & I915_DISPLAY_PIPE_EVENT_INTERRUPT(pipe)) > > + mask |=3D dev_priv->pipestat_irq_mask[pipe]; > > + pipe_stats[pipe] &=3D mask; > > + > > + if (pipe_stats[pipe] & (PIPE_FIFO_UNDERRUN_STATUS | > > + PIPESTAT_INT_STATUS_MASK)) > > I915_WRITE(reg, pipe_stats[pipe]); > > } > > spin_unlock(&dev_priv->irq_lock); > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i91= 5_reg.h > > index c998c4f..47e0635 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -997,6 +997,10 @@ > > #define I915_DISPLAY_PIPE_A_EVENT_INTERRUPT (1<<6) > > #define I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT (1<<5) > > #define I915_DISPLAY_PIPE_B_EVENT_INTERRUPT (1<<4) > > +#define I915_DISPLAY_PIPE_EVENT_INTERRUPT(pipe) \ > > + ((pipe) =3D=3D PIPE_A ? I915_DISPLAY_PIPE_A_EVENT_INTERRUPT : \ > > + I915_DISPLAY_PIPE_B_EVENT_INTERRUPT) > > + > > #define I915_DEBUG_INTERRUPT (1<<2) > > #define I915_USER_INTERRUPT (1<<1) > > #define I915_ASLE_INTERRUPT (1<<0) > > --=20 > > 1.8.4 > >=20 > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >=20 --=-iFD2J3utLXeE53oYsjBF 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.12 (GNU/Linux) iQEcBAABAgAGBQJS8lcyAAoJEORIIAnNuWDFo1oIAMuURe6ys5Ya4xTL04iq5SpF 6BtZeOKKTiFPc0x4uhNape3RwXHW2HWWxgcJ9dk9/BjnRF9/ntsmaQPYChqqKfBU TQStmJxCJziozcjM5urPTrfrZbpuv0zlycCzfMg5xlKzCa1RHSm2tFlrVuDblU5S qUqeHL+VgMW3qnBC6XFFFfVkqqJ+Td1NHEKuLPIwn0RBFw9rHQqgiYWq/SUtbhF6 uRyM0heuyljUHRMH/cOJ3949TN6NFdSJiyFsPJ33uSgQdH7qfCe8CAb+1F+/4hi0 9mpalOfGnlYfxdVro2VkvBS3zG/HWAx6LHqbbUDYSjweLzpBmjDIv9jBGzXiGVA= =dHtN -----END PGP SIGNATURE----- --=-iFD2J3utLXeE53oYsjBF-- --===============0328997862== 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 --===============0328997862==--