Re: [PATCH 00/16] Atomic/nuclear modeset/pageflip

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Mar 20, 2014 at 03:34:19PM -0700, Greg Hackmann wrote:
> On Wed, Mar 19, 2014 at 5:23 AM, Rob Clark <robdclark@gmail.com> wrote:
> 
> > > Hm, do you have some pointers to read up on this? I still think a more
> > > elaborate fail scheme is overkill, but maybe reading a bit of android
> > code
> > > convinces me differently ;-)
> >
> > sadly no pointers to anything to read (but ofc would be interested if
> > anyone else does)..
> >
> 
> The hardware composer HAL is probably the best reference for what Android
> needs from display.  It's defined and documented here (see struct
> hwc_composer_device_1):
> 
> *https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/hwcomposer.h
> <https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/hwcomposer.h>*
> 
> (You can disregard any HWC 1.0 specific behavior in that documentation,
> that's only around for legacy reasons and we strongly encourage vendors not
> to implement HWC 1.0 anymore.)
> 
> Vendors need to ship a compliant HWC HAL implementation on their devices.
>  We don't mandate anything about their userspace/kernel interface beyond
> that.
> 
> 
> >  * I kinda would still like to add to the atomic ioctl some way to indicate
> >    ok/error on a bit finer granularity than per-ioctl.  Ie. perhaps an
> >    array where userspace can ask for results per KMS object (crtc/plane)?
> >    Something along these lines would give the kernel a bit more flexibility
> >    to deal with some of the edge cases which come up when you gang crtcs
> >    for ultra high resolution displays.  In short, it would let the driver
> >    "steal" some of the planes if needed from userspace.
> >
> >    This would also, conveniently, be pretty similar to how (AFAIU) hw
> >    compositor and ADF work on android.  Which seems like it would be useful
> >    to eventually enable android devices to use an upstream display driver.
> >
> >    For this we could pretty easily just throw in a placeholder that we
> >    could replace later with an optional ptr to __user array.  I think
> >    that would be fine for an initial version, but I just wanted to throw
> >    this idea out there, because I think it will become important.
> >
> 
> SurfaceFlinger needs the HWC HAL to return a retire fence fd on each flip,
> for each display.  Per-interface return data would be a good fit for this,
> but per-CRTC or per-plane would work too (userspace could assemble a
> per-interface fence by merging finer-grained fences).
> 
> It's up to the vendor whether their HWC HAL does fine-grained error
> handling and renegotiation like you're describing.  Typically they don't,
> they just push that complexity into userspace instead.  e.g. if the display
> controller needs to gang together two planes for a given configuration, the
> HWC HAL's prepare() op will already know to set aside that extra plane and
> plan accordingly.

I've read through the hwc interface stuff and some question only
marginally related to atomic pageflips poppped up.

- You have an explicit callback for vblank events (well, just the
  interrupt, afaics no support to get a vblank event for a specific frame
  in the future). Any reason not to use android syncpoint fences for this
  like everywhere else? Or is the idea behind all the fences hwc throws
  around just to synchronize read/write access from
  cpu/render/codec/whatever and not so much timing?

- If I read this correct you mandate that the fences fields gets filled
  out in the prepare hook, i.e. before we commit the new state to
  hardware. You also mandate that the fences get eventually signalled even
  when surfaceflinger decides to not commit the new state. I see two
  consequences from that:

  - You create fences before the kernel is committed to execute the
    relevant event which will trigger the fence.
  - Userspace seems to be required to trigger fences manually to unblock
    things in case the hw state isn't committed.

  This seems to be squarely at odds with the hw fences Maarten Lankhorst,
  which currently have two guarantees:
  - Fences can only be created when the kernel commits to execute the
    event that will trigger the fence.
  - A global ordering is imposed upon all fence updates (and so scheduled
    depencies when one fences needs to wait for another).

  Together this ensures that we never schedule a deadlock in hw fences and
  that no hw fence ever depends upon userspace signalling some fences.

  How do you tackle these issues with surfaceflinger/hwc? Or if this blows
  up and the hw deadlocks it's the integrates problem to track it down (or
  just reset the gpu and everything else to get unstuck)?

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