From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 6/7] drm/i915: Optimize gen8_enable|disable_vblank functions Date: Thu, 7 Nov 2013 15:37:54 +0200 Message-ID: <20131107133754.GO5986@intel.com> References: <1383818746-1199-1-git-send-email-daniel.vetter@ffwll.ch> <1383818746-1199-6-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id A5281FB49E for ; Thu, 7 Nov 2013 05:37:58 -0800 (PST) Content-Disposition: inline In-Reply-To: <1383818746-1199-6-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 On Thu, Nov 07, 2013 at 11:05:45AM +0100, Daniel Vetter wrote: > Let's cache the IMR value like on other platforms. This is needed to > implement the underrun reporting since then we'll have two places that > change the same register at runtime. This looks OK, so: Reviewed-by: Ville Syrj=E4l=E4 But I think gen8_de_irq_postinstall() isn't quite right. It'll already enable and unmask the vblank irqs, even though it should just enable them, but leave them masked. > = > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_irq.c | 18 ++++++------------ > 1 file changed, 6 insertions(+), 12 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_= irq.c > index d2d678f72486..51966feee5d2 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2203,17 +2203,14 @@ static int gen8_enable_vblank(struct drm_device *= dev, int pipe) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > unsigned long irqflags; > - uint32_t imr; > = > if (!i915_pipe_enabled(dev, pipe)) > return -EINVAL; > = > spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > - imr =3D I915_READ(GEN8_DE_PIPE_IMR(pipe)); > - if ((imr & GEN8_PIPE_VBLANK) =3D=3D 1) { > - I915_WRITE(GEN8_DE_PIPE_IMR(pipe), imr & ~GEN8_PIPE_VBLANK); > - POSTING_READ(GEN8_DE_PIPE_IMR(pipe)); > - } > + dev_priv->de_irq_mask[pipe] &=3D ~GEN8_PIPE_VBLANK; > + I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]); > + POSTING_READ(GEN8_DE_PIPE_IMR(pipe)); > spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > return 0; > } > @@ -2270,17 +2267,14 @@ static void gen8_disable_vblank(struct drm_device= *dev, int pipe) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > unsigned long irqflags; > - uint32_t imr; > = > if (!i915_pipe_enabled(dev, pipe)) > return; > = > spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > - imr =3D I915_READ(GEN8_DE_PIPE_IMR(pipe)); > - if ((imr & GEN8_PIPE_VBLANK) =3D=3D 0) { > - I915_WRITE(GEN8_DE_PIPE_IMR(pipe), imr | GEN8_PIPE_VBLANK); > - POSTING_READ(GEN8_DE_PIPE_IMR(pipe)); > - } > + dev_priv->de_irq_mask[pipe] |=3D GEN8_PIPE_VBLANK; > + I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]); > + POSTING_READ(GEN8_DE_PIPE_IMR(pipe)); > spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > } > = > -- = > 1.8.4.rc3 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC