Re: [PATCH] drm/i915: FBC flush nuke for BDW

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Aug 07, 2014 at 01:04:19PM -0700, Rodrigo Vivi wrote:
> I tested here on HSW a full sw nuke/cache clean and I didn't liked the
> result.
> It seems to compress less than the hw one and to recompress everything a
> lot and stay less time compressed.

That is really unexpected. For a modern desktop (i.e. anything that
pageflips) there should be zero difference. And for actual frontbuffer
rendering there should only be a difference when doing tiny cpu rendering
to the frontbuffer.

So something didn't work out as expected. Can you please push the code
somewhere, or just submit patches to intel-gfx?

Thanks, Daniel
> 
> So, imho v3 is the way to go.
> 
> 
> On Mon, Aug 4, 2014 at 3:51 AM, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> 
> > According to spec FBC on BDW and HSW are identical without any gaps.
> > So let's copy the nuke and let FBC really start compressing stuff.
> >
> > Without this patch we can verify with false color that nothing is being
> > compressed. With the nuke in place and false color it is possible
> > to see false color debugs.
> >
> > Unfortunatelly on some rings like BCS on BDW we have to avoid Bits 22:18 on
> > LRIs due to a high risk of hung. So, when using Blt ring for frontbuffer
> > rend
> > cache would never been cleaned and FBC would stop compressing buffer.
> > One alternative is to cache clean on software frontbuffer tracking.
> >
> > v2: Fix rebase conflict.
> > v3: Do not clean cache on BCS ring. Instead use sw frontbuffer tracking.
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h         |  1 +
> >  drivers/gpu/drm/i915/intel_display.c    |  3 +++
> >  drivers/gpu/drm/i915/intel_pm.c         | 10 ++++++++++
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 10 +++++++++-
> >  4 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 2a372f2..25d7365 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2713,6 +2713,7 @@ extern void intel_modeset_setup_hw_state(struct
> > drm_device *dev,
> >  extern void i915_redisable_vga(struct drm_device *dev);
> >  extern void i915_redisable_vga_power_on(struct drm_device *dev);
> >  extern bool intel_fbc_enabled(struct drm_device *dev);
> > +extern void gen8_fbc_sw_flush(struct drm_device *dev, u32 value);
> >  extern void intel_disable_fbc(struct drm_device *dev);
> >  extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
> >  extern void intel_init_pch_refclk(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 883af0b..c8421cd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9044,6 +9044,9 @@ void intel_frontbuffer_flush(struct drm_device *dev,
> >         intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
> >
> >         intel_edp_psr_flush(dev, frontbuffer_bits);
> > +
> > +       if (IS_GEN8(dev))
> > +               gen8_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
> >  }
> >
> >  /**
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 684dc5f..de07d3e 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -345,6 +345,16 @@ bool intel_fbc_enabled(struct drm_device *dev)
> >         return dev_priv->display.fbc_enabled(dev);
> >  }
> >
> > +void gen8_fbc_sw_flush(struct drm_device *dev, u32 value)
> > +{
> > +       struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> > +       if (!IS_GEN8(dev))
> > +               return;
> > +
> > +       I915_WRITE(MSG_FBC_REND_STATE, value);
> > +}
> > +
> >  static void intel_fbc_work_fn(struct work_struct *__work)
> >  {
> >         struct intel_fbc_work *work =
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 2908896..2fe871c 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -406,6 +406,7 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
> >  {
> >         u32 flags = 0;
> >         u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
> > +       int ret;
> >
> >         flags |= PIPE_CONTROL_CS_STALL;
> >
> > @@ -424,7 +425,14 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
> >                 flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
> >         }
> >
> > -       return gen8_emit_pipe_control(ring, flags, scratch_addr);
> > +       ret = gen8_emit_pipe_control(ring, flags, scratch_addr);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (!invalidate_domains && flush_domains)
> > +               return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
> > +
> > +       return 0;
> >  }
> >
> >  static void ring_write_tail(struct intel_engine_cs *ring,
> > --
> > 1.9.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

> _______________________________________________
> 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