From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rodrigo Vivi Subject: Re: [PATCH v3 06/10] drm/i915: Implement LRI based FBC tracking Date: Wed, 20 Nov 2013 14:55:57 -0800 Message-ID: <20131120225249.GD4234@bratislava.jf.intel.com> References: <1383771745-22463-1-git-send-email-ville.syrjala@linux.intel.com> <1383771745-22463-7-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-pb0-f44.google.com (mail-pb0-f44.google.com [209.85.160.44]) by gabe.freedesktop.org (Postfix) with ESMTP id 91E95FB67B for ; Wed, 20 Nov 2013 14:55:57 -0800 (PST) Received: by mail-pb0-f44.google.com with SMTP id rq2so4175734pbb.31 for ; Wed, 20 Nov 2013 14:55:57 -0800 (PST) Content-Disposition: inline In-Reply-To: <1383771745-22463-7-git-send-email-ville.syrjala@linux.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.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Nov 06, 2013 at 11:02:21PM +0200, ville.syrjala@linux.intel.com wro= te: > From: Ville Syrj=E4l=E4 > = > As per the SNB and HSW PM guides, we should enable FBC render/blitter > tracking only during batches targetting the front buffer. You improved things a lot here, but I'm just not convinced this is tracking= only and all front buffer touches. > = > On SNB we must also update the FBC render tracking address whenever it > changes. And since the register in question is stored in the context, > we need to make sure we reload it with correct data after context > switches. > = > On IVB/HSW we use the render nuke mechanism, so no render tracking > address updates are needed. Hoever on the blitter side we need to > enable the blitter tracking like on SNB, and in addition we need > to issue the cache clean messages, which we already did. > = > v2: Introduce intel_fb_obj_has_fbc() > Fix crtc locking around crtc->fb access > Drop a hunk that was included by accident in v1 > Set fbc_address_dirty=3Dfalse not true after emitting the LRI > v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't > need to upset lockdep anymore > = > Signed-off-by: Ville Syrj=E4l=E4 > --- > drivers/gpu/drm/i915/i915_gem_context.c | 7 ++++ > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31 ++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 17 +++++++-- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_ringbuffer.c | 58 ++++++++++++++++++++++++= +++++- > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++ > 6 files changed, 113 insertions(+), 3 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i9= 15/i915_gem_context.c > index 72a3df3..d438ea1 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -404,6 +404,13 @@ mi_set_context(struct intel_ring_buffer *ring, > = > intel_ring_advance(ring); > = > + /* > + * FBC RT address is stored in the context, so we may have just > + * restored it to an old value. Make sure we emit a new LRI > + * to update the address. > + */ > + ring->fbc_address_dirty =3D true; > + > return ret; > } > = > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm= /i915/i915_gem_execbuffer.c > index 885d595..db25158 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -886,6 +886,35 @@ validate_exec_list(struct drm_i915_gem_exec_object2 = *exec, > } > = > static void > +i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring, > + struct list_head *vmas) > +{ > + struct i915_vma *vma; > + struct drm_i915_gem_object *fbc_obj =3D NULL; > + u32 fbc_address =3D -1; > + > + list_for_each_entry(vma, vmas, exec_list) { > + struct drm_i915_gem_object *obj =3D vma->obj; > + > + if (obj->base.pending_write_domain && > + intel_fb_obj_has_fbc(obj)) { > + WARN_ON(fbc_obj && fbc_obj !=3D obj); > + fbc_obj =3D obj; > + } > + } > + > + if (fbc_obj) > + fbc_address =3D i915_gem_obj_ggtt_offset(fbc_obj); > + > + /* need to nuke/cache_clean on IVB+? */ > + ring->fbc_dirty =3D fbc_obj !=3D NULL; > + > + /* need to update FBC tracking? */ > + ring->fbc_address_dirty =3D fbc_address !=3D ring->fbc_address; > + ring->fbc_address =3D fbc_address; > +} > + > +static void > i915_gem_execbuffer_move_to_active(struct list_head *vmas, > struct intel_ring_buffer *ring) > { > @@ -1150,6 +1179,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void= *data, > if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping) > i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level); > = > + i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas); > + > ret =3D i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas); > if (ret) > goto err; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index bce6e07..c29e9d4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8074,6 +8074,21 @@ void intel_mark_idle(struct drm_device *dev) > gen6_rps_idle(dev->dev_private); > } > = > +bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj) > +{ > + struct drm_device *dev =3D obj->base.dev; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + > + /* check for potential scanout */ > + if (!obj->pin_display) > + return false; > + > + if (!dev_priv->fbc.fb) > + return false; > + > + return to_intel_framebuffer(dev_priv->fbc.fb)->obj =3D=3D obj; > +} > + > void intel_mark_fb_busy(struct drm_i915_gem_object *obj, > struct intel_ring_buffer *ring) > { > @@ -8091,8 +8106,6 @@ void intel_mark_fb_busy(struct drm_i915_gem_object = *obj, > continue; > = > intel_increase_pllclock(crtc); > - if (ring && intel_fbc_enabled(dev)) > - ring->fbc_dirty =3D true; > } > } > = > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/inte= l_drv.h > index 6d701e7..5c7e8b4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -614,6 +614,7 @@ void intel_ddi_get_config(struct intel_encoder *encod= er, > /* intel_display.c */ > int intel_pch_rawclk(struct drm_device *dev); > void intel_mark_busy(struct drm_device *dev); > +bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj); > void intel_mark_fb_busy(struct drm_i915_gem_object *obj, > struct intel_ring_buffer *ring); > void intel_mark_idle(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i9= 15/intel_ringbuffer.c > index 4649bf5..64fbab5 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -207,6 +207,57 @@ intel_emit_post_sync_nonzero_flush(struct intel_ring= _buffer *ring) > return 0; > } > = > +static int gen6_blt_fbc_tracking(struct intel_ring_buffer *ring) > +{ > + int ret; > + > + if (!ring->fbc_address_dirty) > + return 0; > + > + ret =3D intel_ring_begin(ring, 4); > + if (ret) > + return ret; > + > + intel_ring_emit(ring, MI_NOOP); > + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); > + intel_ring_emit(ring, GEN6_BLITTER_ECOSKPD); > + if (ring->fbc_address !=3D -1) > + intel_ring_emit(ring, _MASKED_BIT_ENABLE(GEN6_BLITTER_FBC_NOTIFY)); > + else > + intel_ring_emit(ring, _MASKED_BIT_DISABLE(GEN6_BLITTER_FBC_NOTIFY)); > + intel_ring_advance(ring); > + > + ring->fbc_address_dirty =3D false; > + > + return 0; > +} > + > +static int gen6_render_fbc_tracking(struct intel_ring_buffer *ring) > +{ > + int ret; > + > + if (!ring->fbc_address_dirty) > + return 0; > + > + ret =3D intel_ring_begin(ring, 4); > + if (ret) > + return ret; > + > + intel_ring_emit(ring, MI_NOOP); > + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); > + intel_ring_emit(ring, ILK_FBC_RT_BASE); > + if (ring->fbc_address !=3D -1) > + intel_ring_emit(ring, ring->fbc_address | > + SNB_FBC_FRONT_BUFFER | ILK_FBC_RT_VALID); > + else > + intel_ring_emit(ring, 0); > + intel_ring_advance(ring); > + > + ring->fbc_address_dirty =3D false; > + > + return 0; > +} > + > static int > gen6_render_ring_flush(struct intel_ring_buffer *ring, > u32 invalidate_domains, u32 flush_domains) > @@ -256,6 +307,9 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring, > intel_ring_emit(ring, 0); > intel_ring_advance(ring); > = > + if (invalidate_domains) > + return gen6_render_fbc_tracking(ring); > + > return 0; > } > = > @@ -1839,7 +1893,9 @@ static int gen6_ring_flush(struct intel_ring_buffer= *ring, > } > intel_ring_advance(ring); > = > - if (IS_GEN7(dev) && !invalidate && flush) > + if (invalidate) > + return gen6_blt_fbc_tracking(ring); > + else if (flush && IS_GEN7(dev)) > return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN); > = > return 0; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i9= 15/intel_ringbuffer.h > index 71a73f4..1e5bbd6 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -143,8 +143,10 @@ struct intel_ring_buffer { > */ > struct drm_i915_gem_request *preallocated_lazy_request; > u32 outstanding_lazy_seqno; > + u32 fbc_address; > bool gpu_caches_dirty; > bool fbc_dirty; > + bool fbc_address_dirty; > = > wait_queue_head_t irq_queue; > = > -- = > 1.8.1.5 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx