Re: [PATCH 00/15] Accurate frontbuffer tracking and psr conversion

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 00/15] Accurate frontbuffer tracking and psr conversion
Date: Tue, 17 Jun 2014 11:30:56 +0200	[thread overview]
Message-ID: <20140617093056.GH5821@phenom.ffwll.local> (raw)
In-Reply-To: <20140617070947.GL17744@nuc-i3427.alporthouse.com>

On Tue, Jun 17, 2014 at 08:09:47AM +0100, Chris Wilson wrote:
> On Mon, Jun 16, 2014 at 07:51:20PM +0200, Daniel Vetter wrote:
> > Hi all,
> > 
> > This patch series adds accurate frontbuffer tracking to i915 and then converts
> > psr over to use. Bunch of things still need to be done.
> > 
> > - PSR needs to be re-tested. I lack the hardware for that. The frontbuffer
> >   tracking itself is tested.
> > 
> > - PSR igt testcase needs to be extended so that we cover all upload methods on
> >   all plane types.
> > 
> > - DRRS/downclocking needs to be unified and put into this framework properly.
> >   Also needs proper locking for all of the DRRS state.
> > 
> > - fbc also needs to be fixed up, with state handling properly split between
> >   crtc_enable/disable, primary fb updates and the fb tracking for nuking. A bit
> >   unclear how we want to integrate gtt cpu write tracking through the hw, since
> >   that seems to be the hw tracking piece that actually works.
> > 
> > General blueprint for display runtime power saving features:
> > - Have all your state in either intel_crtc or dev_priv, protected by its own
> >   lock.
> > 
> > - Do state setup in crtc_enable, cleanup in crtc_disable with a pair of
> >   enable/disable fuctions. Optionally update everywhere else you want (e.g.
> >   primary plane updates for fbc). Not state setup anywhere else allowed, except
> >   maybe an _init for setting up work items, locks, ...
> > 
> > - Intercept the invalidation/flush signals you need like
> >   psr_invalidate/psr_flush. Track internally which frontbuffer bits you're
> >   interested in and invalidate/flush accordingly. You can also use these for
> >   workarounds (e.g. on hsw we force-flush for sprite changes since the flip
> >   signal doesn't result in a hw image upload).
> > 
> > 
> > Note that currently the gtt tracking has a bit a gap: We never exit it. Bunch of
> > fixes are possible:
> > - Wire up the core dirty_fb ioctl to flush framebuffers. This is used by generic
> >   userspace to flush dummy buffers, which in our case means gtt mmaps. So maps
> >   perfectly.
> > 
> > - Do a delayed gtt pte teardown and force-flush. Probably too ugly to care
> >   really.
> > 
> > - Try to integrate hw gtt write tracking logic. Note that current psr code
> >   doesn't rely on this - I've killed the X-tiled checks completely.
> > 
> > Big thanks to Chris for some great ideas for the fb tracking scheme and the
> > precise placement of the invalidate/flush functions.
> > 
> > Comments, flames and especially testing on psr hardware highly welcome.
> 
> I like it. I think this is a big step in the right direction, and
> doesn't look too intrusive to normal operations. I would like to get
> Ville's FBC tracking on top so we check for any idiosynacracies we may
> have missed.

My plan is to check out how this works with byt psr (since the hw tracking
there is completely broken) and then get this all in. More polish might be
needed on top for fbc, but fixing fbc will be a lot of work (and Ville's
going on summer vacation soonish). Also with that we could fix up fbc and
drrs in parallel.

I hope that byt psr is a nasty enough case to validate the framework
overall.

Anyway, tons of changes in the patches from Chris' and Rodrigo's review
plus fixing a bunch of things when debugging psr with Rodrigo yesterday.
Unfortunately the psr sink crc tests are broken currently (for unrelated
reasons) so the verdict's still out on whether it works.

New patch series pushed to

http://cgit.freedesktop.org/~danvet/drm/log/?h=psr-stuff

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch