From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/2] drm/i915: Fix scanline counter fixup on BDW Date: Tue, 11 Mar 2014 14:01:49 +0200 Message-ID: <20140311120149.GB13659@intel.com> References: <1394535526-20513-1-git-send-email-ville.syrjala@linux.intel.com> <1394535526-20513-2-git-send-email-ville.syrjala@linux.intel.com> <20140311113810.GE30571@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id C061CFAAE9 for ; Tue, 11 Mar 2014 05:02:02 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140311113810.GE30571@phenom.ffwll.local> 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-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Mar 11, 2014 at 12:38:10PM +0100, Daniel Vetter wrote: > On Tue, Mar 11, 2014 at 12:58:46PM +0200, ville.syrjala@linux.intel.com w= rote: > > From: Ville Syrj=E4l=E4 > > = > > The display interrupts changed on BDW, so the current ILK-HSW specific > > code in ilk_pipe_in_vblank_locked() doesn't work there. Add the required > > bits for BDW, and while at it, change the existing code to use nicer > > looking vblank status bit macros. > > = > > Also remove the now stale __raw_i915_read16() definition which was > > left over from the failed gen2 ISR experiment. > > = > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3D73962 > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/i915_irq.c | 30 +++++++++++------------------- > > 1 file changed, 11 insertions(+), 19 deletions(-) > > = > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i91= 5_irq.c > > index d4c952d..ec9b8a4 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -618,33 +618,25 @@ static u32 gm45_get_vblank_counter(struct drm_dev= ice *dev, int pipe) > > = > > /* raw reads, only for fast reads of display block, no need for forcew= ake etc. */ > = > fast reads in context with mmio transactions sounds like a fairly crazy > oxymoron. And our I915_READ functions should already dtrt wrt not doing > the forcewake dance for display registers, otherwise I'll consider it a > bug. > = > Care to throw a patch on top to remove these guys completely? We could have other junk in there like unclaimed register access checks which could slow this down a bit. Although it seems we don't do unclaimed register checks for reads currently. I'm going to kill the ISR read anyway in the atomic sprite series, so that'll get us back to just one mmio read per call, and so even if we did a few extra mmio accesses for unclaimed register access checks, it doesn't feel too expensive to me. So yes, I could throw on an extra patch to do what you ask, but I'd rather do that after the atomic sprite series is applied. These patches are meant for -fixes anyway, so I want to keep them fairly minimal. > Thanks, Daniel > = > > #define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs = + (reg__)) > > -#define __raw_i915_read16(dev_priv__, reg__) readw((dev_priv__)->regs = + (reg__)) > > = > > static bool ilk_pipe_in_vblank_locked(struct drm_device *dev, enum pip= e pipe) > > { > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > uint32_t status; > > - > > - if (INTEL_INFO(dev)->gen < 7) { > > - status =3D pipe =3D=3D PIPE_A ? > > - DE_PIPEA_VBLANK : > > - DE_PIPEB_VBLANK; > > + int reg; > > + > > + if (INTEL_INFO(dev)->gen >=3D 8) { > > + status =3D GEN8_PIPE_VBLANK; > > + reg =3D GEN8_DE_PIPE_ISR(pipe); > > + } else if (INTEL_INFO(dev)->gen >=3D 7) { > > + status =3D DE_PIPE_VBLANK_IVB(pipe); > > + reg =3D DEISR; > > } else { > > - switch (pipe) { > > - default: > > - case PIPE_A: > > - status =3D DE_PIPEA_VBLANK_IVB; > > - break; > > - case PIPE_B: > > - status =3D DE_PIPEB_VBLANK_IVB; > > - break; > > - case PIPE_C: > > - status =3D DE_PIPEC_VBLANK_IVB; > > - break; > > - } > > + status =3D DE_PIPE_VBLANK(pipe); > > + reg =3D DEISR; > > } > > = > > - return __raw_i915_read32(dev_priv, DEISR) & status; > > + return __raw_i915_read32(dev_priv, reg) & status; > > } > > = > > static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, > > -- = > > 1.8.3.2 > > = > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > = > -- = > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- = Ville Syrj=E4l=E4 Intel OTC