From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v2 1/3] drm/i915: Skip register reads in i915_get_crtc_scanoutpos() Date: Tue, 24 Sep 2013 11:11:27 +0200 Message-ID: <20130924091127.GE13668@phenom.ffwll.local> References: <20130923103050.GE8482@nuc-i3427.alporthouse.com> <1379936930-26044-1-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ea0-f175.google.com (mail-ea0-f175.google.com [209.85.215.175]) by gabe.freedesktop.org (Postfix) with ESMTP id D69F6E5DCD for ; Tue, 24 Sep 2013 02:11:10 -0700 (PDT) Received: by mail-ea0-f175.google.com with SMTP id m14so2329399eaj.34 for ; Tue, 24 Sep 2013 02:11:10 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1379936930-26044-1-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org, Mario Kleiner List-Id: intel-gfx@lists.freedesktop.org On Mon, Sep 23, 2013 at 02:48:50PM +0300, ville.syrjala@linux.intel.com wro= te: > From: Ville Syrj=E4l=E4 > = > We have all the information we need in the mode structure, so going and > reading it from the hardware is pointless, and slower. > = > We never populated ->get_vblank_timestamp() in the UMS case, and as that > is the only way we'd ever call ->get_scanout_position(), we can > completely ignore UMS in i915_get_crtc_scanoutpos(). > = > Also reorganize intel_irq_init() a bit to clarify the KMS vs. UMS > situation. > = > v2: Drop UMS code > = > Cc: Mario Kleiner > Signed-off-by: Ville Syrj=E4l=E4 > --- > drivers/gpu/drm/i915/i915_irq.c | 43 ++++++++++++++++-------------------= ------ > 1 file changed, 17 insertions(+), 26 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_= irq.c > index b356dc1..058f099 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -570,24 +570,29 @@ static u32 gm45_get_vblank_counter(struct drm_devic= e *dev, int pipe) > static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, > int *vpos, int *hpos) > { > - drm_i915_private_t *dev_priv =3D (drm_i915_private_t *) dev->dev_privat= e; > - u32 vbl =3D 0, position =3D 0; > + 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); > + const struct drm_display_mode *mode =3D &intel_crtc->config.adjusted_mo= de; > + u32 position; > int vbl_start, vbl_end, htotal, vtotal; > bool in_vbl =3D true; > int ret =3D 0; > - enum transcoder cpu_transcoder =3D intel_pipe_to_cpu_transcoder(dev_pri= v, > - pipe); Hooray, this rips out the racy pipe_to_cpu_transcoder deref, so I'm all in favour \o/ Of course I still have that ugly itme on my todo about "fix the locking for kms vblank stuff" ;-) Mario, can you please take a look at these patches and ack them? I'd like to slurp them in for -next. -Daniel > = > - if (!i915_pipe_enabled(dev, pipe)) { > + if (!intel_crtc->active) { > DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled " > "pipe %c\n", pipe_name(pipe)); > return 0; > } > = > - /* Get vtotal. */ > - vtotal =3D 1 + ((I915_READ(VTOTAL(cpu_transcoder)) >> 16) & 0x1fff); > + htotal =3D mode->crtc_htotal; > + vtotal =3D mode->crtc_vtotal; > + vbl_start =3D mode->crtc_vblank_start; > + vbl_end =3D mode->crtc_vblank_end; > = > - if (INTEL_INFO(dev)->gen >=3D 4) { > + ret |=3D DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE; > + > + if (IS_G4X(dev) || INTEL_INFO(dev)->gen >=3D 5) { > /* No obvious pixelcount register. Only query vertical > * scanout position from Display scan line register. > */ > @@ -605,29 +610,16 @@ static int i915_get_crtc_scanoutpos(struct drm_devi= ce *dev, int pipe, > */ > position =3D (I915_READ(PIPEFRAMEPIXEL(pipe)) & PIPE_PIXEL_MASK) >> PI= PE_PIXEL_SHIFT; > = > - htotal =3D 1 + ((I915_READ(HTOTAL(cpu_transcoder)) >> 16) & 0x1fff); > *vpos =3D position / htotal; > *hpos =3D position - (*vpos * htotal); > } > = > - /* Query vblank area. */ > - vbl =3D I915_READ(VBLANK(cpu_transcoder)); > - > - /* Test position against vblank region. */ > - vbl_start =3D vbl & 0x1fff; > - vbl_end =3D (vbl >> 16) & 0x1fff; > - > - if ((*vpos < vbl_start) || (*vpos > vbl_end)) > - in_vbl =3D false; > + in_vbl =3D *vpos >=3D vbl_start && *vpos < vbl_end; > = > /* Inside "upper part" of vblank area? Apply corrective offset: */ > if (in_vbl && (*vpos >=3D vbl_start)) > *vpos =3D *vpos - vtotal; > = > - /* Readouts valid? */ > - if (vbl > 0) > - ret |=3D DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE; > - > /* In vblank? */ > if (in_vbl) > ret |=3D DRM_SCANOUTPOS_INVBL; > @@ -3148,11 +3140,10 @@ void intel_irq_init(struct drm_device *dev) > dev->driver->get_vblank_counter =3D gm45_get_vblank_counter; > } > = > - if (drm_core_check_feature(dev, DRIVER_MODESET)) > + if (drm_core_check_feature(dev, DRIVER_MODESET)) { > dev->driver->get_vblank_timestamp =3D i915_get_vblank_timestamp; > - else > - dev->driver->get_vblank_timestamp =3D NULL; > - dev->driver->get_scanout_position =3D i915_get_crtc_scanoutpos; > + dev->driver->get_scanout_position =3D i915_get_crtc_scanoutpos; > + } > = > if (IS_VALLEYVIEW(dev)) { > dev->driver->irq_handler =3D valleyview_irq_handler; > -- = > 1.8.1.5 > = > _______________________________________________ > 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