Re: [PATCH] drm/i915: Don't wait for vblank for sprite plane flips

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jul 01, 2013 at 10:56:06AM +0530, Vijay Purushothaman wrote:
> On 6/28/2013 7:54 PM, Ville Syrjälä wrote:
> > On Fri, Jun 28, 2013 at 07:45:31PM +0530, Vijay Purushothaman wrote:
> >> Since the sprite planes are using synchronized MMIO based flip, no need
> >> to wait for vblank. Removing this wait allows us to get a nice
> >> performance boost to both 3D & media workloads based on sprite (~60 fps
> >> from ~20 fps)
> >
> > Nak. We can't unpin the buffer until the hardware has finished reading
> > from it.
> 
> Thanks much for the review feedback. We have removed this check in our 
> android production branch so far we have not seen any side effect or 
> artifact. Is this a conservative check or do we have any use case which 
> will fail without this piece of code?
> 
> Apparently the windows driver team have been using MMIO flips for Sprite 
> and the windows driver also didn't wait for vblank for such flips all along.
> 
> Could you please help me understand this a bit better? This wait reduces 
> the perf to ~20 fps and thus prevent us from using sprite for any OGL 
> layer mapping in hwcomposer and we are losing significant amount of 
> power. For video content playback the problem is not that bad.

I'd say trying to implement hwcomposer w/o atomic pageflip is anyway a
doomed idea. You will see ugly glitches if/when the sprite turns on/off
too late/soon. I actually tried it myself a few years ago, and then
realized that it just won't cut it, which is why I started writing
some of the early atomic page flip code.

> > The proper fix is to do the unpin asynchronously after the flip has
> > completed. That's one part of the bigger atomic pageflip story.
> >
> 
> Any idea when are we planning to merge this atomic flip in d-i-n-q?

After someone has time to rewrite a bunch of the code.

My rough plan:
- Fix watermarks. I've been working on it, and I hope to have something
  reasonable ready before my summer vacation starts after this week.
  Then there's still the pre-pch watermark code to fix.
- Implement drm_plane support for primary planes. I posted some gen2-4
  sprite C code a while back which should serve as a starting point. I
  still haven't quite figured out how we're going to handle compatibility
  w/ old userland though. We may not want to expose these planes unless
  we know userland can handle them. I want to do this before we merge
  any new atomic page flip ioctl, so that we don't have to carry the
  legacy baggage in the new API.
- Cook up some plane_config type of stuff to pre-compute the plane state
- Plug in the real atomic stuff

Another big issues is the locking. We really want to get fine grained
locking for the atomic page flip too, so someone has to figure out how
to do it. Daniel's idea of using the ww_mutex stuff for this sounds like
a reasonable approach to me, but I haven't really thought about it that
much yet. Currently even the non-atomic plane ioctls take the big kms
lock, which plainly sucks.

There are a bunch of API issues left as well, like defining new
properties, figuring out the event stuff (I know some people would
prefer a single even for the whole crtc, which could be done, except
we have to carefully define how it behaves in certain special cases),
and probably some other things I forgot already.

One other interesting idea would be having some swap interval or swap
target timestamp stuff in the kernel, so that userspace could even
schedule multiple atomic updates in advance. This may turn out rather
problematic when performing operations on multiple crtcs, since there
could be several different states in the pipeline for one crtc, so the
state of another crtc would possibly need to be checked against all of
them, so that we can be sure whether the operation will succeed
regardless of which order they end up being pushed to the hardware.
Maybe we'd just limit such heavy pipelining to simple page flip
scenarios, which could neatly avoid some of the problems. But work on
this stuff can easily wait until the basic atomic stuff is in.

-- 
Ville Syrjälä
Intel OTC