From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH] drm/i915: Some polish for the new pipestat_irq_handler Date: Wed, 12 Feb 2014 19:31:16 +0200 Message-ID: <1392226276.3028.3.camel@intelbox> References: <1392222066-11154-1-git-send-email-daniel.vetter@ffwll.ch> <1392224136-19620-1-git-send-email-daniel.vetter@ffwll.ch> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2002722375==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 983E9FAF08 for ; Wed, 12 Feb 2014 09:31:54 -0800 (PST) In-Reply-To: <1392224136-19620-1-git-send-email-daniel.vetter@ffwll.ch> 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: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org --===============2002722375== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-LI0M90V4ZOOUkM1yFSLj" --=-LI0M90V4ZOOUkM1yFSLj Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2014-02-12 at 17:55 +0100, Daniel Vetter wrote: > Just a bit of polish which I hope will help me with massaging some > internal patches to use Imre's reworked pipestat handling: > - Don't check for underrun reporting or enable pipestat interrupts > twice. > - Frob the comments a bit. > - Do the iir PIPE_EVENT to pipe mapping explicitly with a switch. We > only have one place which does this, so better to make it explicit. >=20 > v2: Ville noticed that I've broken the logic a bit with trying to > avoid checking whether we're interested in a given pipe twice. push > the PIPESTAT read down after we've computed the mask of interesting > bits first to avoid that duplication properly. >=20 > Cc: Imre Deak > Cc: Ville Syrj=C3=A4l=C3=A4 > Signed-off-by: Daniel Vetter Looks simpler, so: Reviewed-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++----------= - > drivers/gpu/drm/i915/i915_reg.h | 4 ---- > 2 files changed, 25 insertions(+), 15 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_= irq.c > index 386a640b7c92..a45ed67407bd 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1559,25 +1559,39 @@ static void valleyview_pipestat_irq_handler(struc= t drm_device *dev, u32 iir) > spin_lock(&dev_priv->irq_lock); > for_each_pipe(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); > + u32 mask, iir_bit; > =20 > /* > - * Clear the PIPE*STAT regs before the IIR > + * PIPESTAT bits get signalled even when the interrupt is > + * disabled with the mask bits, and some of the status bits do > + * not generate interrupts at all (like the underrun bit). Hence > + * we need to be careful that we only handle what we want to > + * handle. > */ > mask =3D PIPESTAT_INT_ENABLE_MASK; > if (__cpu_fifo_underrun_reporting_enabled(dev, pipe)) > mask |=3D PIPE_FIFO_UNDERRUN_STATUS; > - if (iir & I915_DISPLAY_PIPE_EVENT_INTERRUPT(pipe)) > + > + switch (pipe) { > + case PIPE_A: > + iir_bit =3D I915_DISPLAY_PIPE_A_EVENT_INTERRUPT; > + break; > + case PIPE_B: > + iir_bit =3D I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; > + break; > + } > + if (iir & iir_bit) > mask |=3D dev_priv->pipestat_irq_mask[pipe]; > - pipe_stats[pipe] &=3D mask; > =20 > + if (!mask) > + continue; > + > + reg =3D PIPESTAT(pipe); > + pipe_stats[pipe] =3D I915_READ(reg) & mask; > + > + /* > + * Clear the PIPE*STAT regs before the IIR > + */ > if (pipe_stats[pipe] & (PIPE_FIFO_UNDERRUN_STATUS | > PIPESTAT_INT_STATUS_MASK)) > I915_WRITE(reg, pipe_stats[pipe]); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_= reg.h > index 645221270c34..8344541bbb93 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -997,10 +997,6 @@ > #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) --=-LI0M90V4ZOOUkM1yFSLj 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) iQEcBAABAgAGBQJS+6/kAAoJEORIIAnNuWDF9EsIAI5Q5vbbogW7MO4omv/bhFOj MzSKCBrOUbvtrbt+ZNmqrslr4V28ZVeKj97KA12FKJtg3kcPKOjBE6D7tNQD490r TL+GBsJPW6l/PL0o46UZeqVWs+vmaBphg2uViRHp57F/5XdqIjsl0Rzldrxtoomg XW865W1By145/y/TXmcMRkyVLgu+GIILeCehsXtWqtX3HOfI9BeYp3Zn+NBXWLIa uz10/k5ReDqf9rOtmdg89gRY7sK5Hwa7TboyLxvbQlxRTswbZukTaUT6En85C/6S 6BIGD6x48I62vA5hSKFGoacMCzVOA3lZD5KrPwxzyFNvVJ1U6TiItmv8+PlRaRI= =EhFu -----END PGP SIGNATURE----- --=-LI0M90V4ZOOUkM1yFSLj-- --===============2002722375== 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 --===============2002722375==--