[Intel-gfx] [PATCH v3 06/10] drm/i915: Implement LRI based FBC tracking

Rodrigo Vivi rodrigo.vivi at gmail.com
Thu Nov 21 00:53:20 CET 2013


actually just ignore my last msg... alternate between gmail and mutt
confused me...



On Wed, Nov 6, 2013 at 1:02 PM,  <ville.syrjala at linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> As per the SNB and HSW PM guides, we should enable FBC render/blitter
> tracking only during batches targetting the front buffer.
>
> 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=false 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älä <ville.syrjala at linux.intel.com>
> ---
>  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/i915/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 = 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 = NULL;
> +       u32 fbc_address = -1;
> +
> +       list_for_each_entry(vma, vmas, exec_list) {
> +               struct drm_i915_gem_object *obj = vma->obj;
> +
> +               if (obj->base.pending_write_domain &&
> +                   intel_fb_obj_has_fbc(obj)) {
> +                       WARN_ON(fbc_obj && fbc_obj != obj);
> +                       fbc_obj = obj;
> +               }
> +       }
> +
> +       if (fbc_obj)
> +               fbc_address = i915_gem_obj_ggtt_offset(fbc_obj);
> +
> +       /* need to nuke/cache_clean on IVB+? */
> +       ring->fbc_dirty = fbc_obj != NULL;
> +
> +       /* need to update FBC tracking? */
> +       ring->fbc_address_dirty = fbc_address != ring->fbc_address;
> +       ring->fbc_address = 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 = 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 = obj->base.dev;
> +       struct drm_i915_private *dev_priv = 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 == 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 = true;
>         }
>  }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_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 *encoder,
>  /* 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/i915/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 = 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 != -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 = false;
> +
> +       return 0;
> +}
> +
> +static int gen6_render_fbc_tracking(struct intel_ring_buffer *ring)
> +{
> +       int ret;
> +
> +       if (!ring->fbc_address_dirty)
> +               return 0;

Anyway I forgot to say that I noticed that even if FBC is disabled it
will do at least once this LRI

> +
> +       ret = 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 != -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 = 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/i915/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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br



More information about the Intel-gfx mailing list