Re: [PATCH 00/25] Fix FBC for real

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jun 18, 2014 at 08:58:33PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This series rewrites the FBC code to actually work. It utilizes the
> hardware tracking/nuking as much as possible, eg. relying on hardware
> nuke on flip when possible.
> 
> I also introduce the generic ring and vblank notifier gizmos which could
> be used for various other things. I already included a patch to convert
> the IPS enable to be asynchronous by using the vblank notifier. Other
> users for thse could be mmio flips, watermark programming, atomic
> gamma/color correction (single buffered registers all) updates from vblank
> interrupt, etc.
> 
> There's also a rather big behavioural change that FBC now stays enabled
> even when multiple pipes are active. FBC just automatially migrates to
> the primary plane where it's deemed most beneficial. We do that
> determination by looking at the rate at which the plane is pulling data
> (pixel rate * cpp). That seems like a reasonable choice all else being
> equal. Eventually we might want to adjust the FBC score based on
> nuke/invalidate frequency as well.
> 
> The locking now has a new fbc.mutex. I had to split the page flip code
> apart a bit to accomondate. I'm not entirely sure if it wouldn't be
> better to just keep struct_mutex locked all through there, but then
> I'd need rework the locking in the fbc code to not take struct_mutex
> when called from the page flip code. And then I'd have to start
> questioning whether fbc.mutex has any point existing.
> 
> I pushed the lot here:
> git://gitorious.org/vsyrjala/linux.git fbc_update_thing_14

Just very cursory look at it, but this conflicts badly (well the
integration with the driver) with my frontbuffer tracking stuff. My idea
was to essentially hide all the fbc integration points we have sprinkled
all over behind that one and maybe even switch to manual invalidation for
gt rendering with a cpu register write and ditch all the complexity we
have with injecting cmds into rings right now.

Other integration points would be an fbc enable called from crtc_enable to
check the basics (like screen size to allocate the fbc backing buffers).
And then an additional function called every time we update the primary
buffer to check for e.g. pixel depth and stuff like that. Post-flip
updates would exclusively happen through the frontbuffer tracking scheme.

Just dumping my high-level thoughts.
-Daniel

> 
> Ville Syrjälä (25):
>   drm/i915: Add ring_notify mechanism
>   drm/i915: Add vblank notify mechanism
>   drm/i915: Name the IPS bits
>   drm/i915: Use vblank notifier for IPS
>   drm/i915: Reogranize page flip code for fbc
>   drm/i915: Move ilk_pipe_pixel_rate() earlier to avoid forward
>     declaration
>   drm/i915: Reorganize intel_update_fbc()
>   drm/i915: Check panel fitting state before enabling fbc
>   drm/i915: Reject fbc on g4x when sprites are enabled
>   drm/i915: Check pixel format for fbc
>   drm/i915: Remove dblscan flag from fbc1 check
>   drm/i915: Don't claim fbc as possible if the obj size exceeds stolen
>     size
>   drm/i915: Use low level funciton to disable fbc at init/resume
>   drm/i915: Move fbc function prototypes got intel_drv.h
>   drm/i915: Move fbc state into dev_priv.fbc
>   drm/i915: Rewrite fbc
>   drm/i915: Reduce dmesg spam from FBC enable
>   drm/i915: Add i915_fbc_info debugfs file
>   drm/i915: Implement LRI based FBC tracking
>   drm/i915: Use LRI based FBC render tracking for ILK
>   drm/i915: Reorder i915_gem_execbuffer_move_to_gpu() and
>     i915_switch_context()
>   drm/i915: Flush caches for scanout during cpu->gtt move
>   drm/i915: Nuke FBC from SW_FINISH ioctl
>   drm/i915: Pimp fbc render/blitter tracking
>   drm/i915: Enable fbc for ilk+ by default
> 
>  drivers/gpu/drm/i915/i915_debugfs.c        |   60 +-
>  drivers/gpu/drm/i915/i915_drv.c            |    3 +
>  drivers/gpu/drm/i915/i915_drv.h            |   44 +-
>  drivers/gpu/drm/i915/i915_gem.c            |   23 +-
>  drivers/gpu/drm/i915/i915_gem_context.c    |   14 +
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   40 +-
>  drivers/gpu/drm/i915/i915_irq.c            |   12 +
>  drivers/gpu/drm/i915/i915_reg.h            |    3 +-
>  drivers/gpu/drm/i915/i915_suspend.c        |    5 +-
>  drivers/gpu/drm/i915/intel_display.c       |  399 ++++++++--
>  drivers/gpu/drm/i915/intel_drv.h           |   30 +-
>  drivers/gpu/drm/i915/intel_pm.c            | 1099 +++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_ringbuffer.c    |  154 +++-
>  drivers/gpu/drm/i915/intel_ringbuffer.h    |   26 +-
>  drivers/gpu/drm/i915/intel_sprite.c        |   15 +-
>  15 files changed, 1483 insertions(+), 444 deletions(-)
> 
> -- 
> 1.8.5.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