[RFC][PATCH 08/10] WIP: drm: Atomic modeset ioctl

[Rob Clark]

On Sat, Sep 1, 2012 at 6:12 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Fri, Aug 31, 2012 at 05:47:13PM -0500, Rob Clark wrote:
>> On Wed, Jun 27, 2012 at 5:24 AM,  <ville.syrjala at linux.intel.com> wrote:
>> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> >
>> > First draft.
>> >
>> > The ioctl simply takes a list of object IDs and property IDs and their
>> > values. For setting values to blob properties, the property value
>> > indicates the length of the data, and the actual data is passed via
>> > another blob pointer.
>> >
>> > Detailed error reporting is missing, as is completion event support.
>>
>> Hmm, I'm thinking we may need to define a bit in the way of
>> semantics..  ie. I think we really do want things like
>> enabling/disabling a plane to be asynchronous (after doing a
>> DRM_ATOMIC_TEST_ONLY call), otherwise performance will suck when
>> switching a window surface to/from a plane.  But maybe we should
>> define that another DRM_IOCTL_MODE_ATOMIC cannot come until after some
>> sort of event back to userspace?
>
> I've discussed things with Kristian Høgsberg (and also Jesse now that he's
> back from sabbatical) a lot and we've concluded on one big thing:
>
>  We should separate the issue of modeset on mutliple crtcs (usually called
>  atomic modeset on irc) from a (usually called nuclear) pageflip on a
>  single crtc that exchanges a bunch of things (framebufferrs, plane
>  parameters, other properties, cursor position, ...).

yeah, nuclear-pageflip would be associated only w/ a single crtc.
Actually I was kinda assuming atomic-modeset was too..  ie. moving a
plane from one crtc to another would be two ioctls, one to remove it
from first crtc, one to add to the other.  Although maybe one big
ioctl is better.

I do think there is a bit of a grey area between the two.  Ie.
enabling/disabling a crtc, or changing what encoder->connector it is
hooked to might be taking several vblank cycles, and is a relatively
infrequent operation so ok to block.  Enabling/disabling a plane could
be much more frequent and should not block.  Returning EINVAL or EBUSY
in the transient case where it isn't ready quite yet seems like a good
approach.. EBUSY might be better for "it might succeed if you try
again in next vblank" and EINVAL for "it won't ever work with current
setup, don't bother trying again".

Infrequent, can be slow:
 + enabling/disabling a new output

Semi-frequent, should not block but -EBUSY ok:
 + enabling/disabling a plane
 + resizing a plane

More frequent, should not block, should not need extra "test" ioctl
step, but should just work
 + page flip, update plane position

So for that middle group, I'm a bit fuzzy on where those operations
should fit in..

> The reason is that these things solve in large parts separate problems:
>
> - atomic modeset is first and foremost a solution to exposing constraints
>   on global configurations due to shared resources like plls, link lanes
>   (e.g. on many intel chipset if you enable multiple outputs you can use
>   the full bw of the single output config) and similar things. The
>   solution is to tell the kernel the entire config and let the kernel
>   decide how to best make it possible with the hw it has (maybe even
>   re-assigning crtcs internally to give the display pipe with the most
>   bandwidth to the crtc with the highest dotclock*bpp requirement). To be
>   able to expose all this to userspace Jesse proposed a test_flag to check
>   configurations without actually changing anything in the hw state. This
>   way a modeset GUI could grey out everything that won't work (since
>   describing the actual hw constraints is too complex, and users don't
>   really care anyway in most cases why something doesn't work).
>
> - a modeset also usually can take a few vblanks, since establishing new
>   clocks and locking them in the various stages of the pipeline can take a
>   while. And since modeset configuration happens seldomly, we imo can
>   simply specify that a modeset always happens synchronously.
>
> - Pageflip on the _must_ happen on a precise vblank, and be fully
>   asynchronous, always. Furthermore we want a timestamp to know when the
>   pageflip actually happened. Again, due to funny hw constraints we might
>   need a test_flag so that a compositor can figure out whether it can use
>   a plane for a given surface. Compared to the atomic modeset test-flag we
>   have a few complications:
>   - We don't want to walk the plane configuration space in the test mode
>     for every frame. Hence the compositor will just try the current
>     configuration (maybe with some surfaces moved around and a few
>     properties changed). If a given plane config doesn't work out, the
>     compositor could just fall back to gpu rendering and try to come up
>     with a new optimal configuration for the next frame.
>   - There's the issue that some changes require more than one vblank to
>     take effect due to some prep work (e.g. switching a plane to a
>     different crtc). My idea here is that the kernel could return a
>     special "transient -EINVAL" for this case indicating to the compositor
>     that it can't do this configuration for the next vblank, but it has
>     set up things so that eventually (in a few vblanks) the sprite will be
>     available on that crtc. The userspace compositor should then just keep
>     on retrying for each frame with this configuration (until it either
>     works or the kernel returns a "definit -EINVAL" indicating that
>     something changed and it can't do this configuration). That way we
>     should be able to support any steady-state plane/fb/cursor/whatever
>     configuration that the hw can, without forcing the compositor to miss
>     a frame. Obviously, these prepare steps _must_ _not_ result in a rouge
>     plane eventually appearing ;-)
>
> - Also a rather practical one: The lack of nuclear pageflip is the reason
>   that Wayland/Weston can't use sprites atm (since set_plane is can be
>   synchronous and can't be synced with a pageflip). Kristian therefore
>   wants the atomic modeset (which I think is much more invasive) not to
>   stall the nuclear pageflip support. And the implementation effort is imo
>   really a big difference: For i915.ko I expect the nuclear pageflip to
>   mostly boil down to wiring up async plane/cursor updates through the
>   latch register (with keeping all the other setup code around since we
>   only allow at most one outstanding pageflip per crtc currently anyway)
>   and wiring that up with the existing crtc pageflip handler. atomic
>   modeset otoh requires an invasive rewrite of the driver code to properly
>   handle these shared resources (and support the test mode). The beginning
>   of that is happening with the modeset rework, but that's just the first
>   step.

Hmm, I was thinking Ville's atomic fxns would be useful, but maybe I
can still bits from that and try and put nuclear-pageflip beneath
atomic-modeset in the stack.

BR,
-R

> I.e. I'm voting to reduce the scope of this ioctl a bit in the naming of
> gaining simpler&clearer semantics. In any case I don't see the big&scary
> work in the ioctl itself, but the actual low-level hw implementation in
> drivers (imo the crtc helper framework is holy inadequate for this safe
> for the simplest of drivers) and in testing the ioctl interfaces
> exhaustively.
>
> Especially the testing is very important, since the entire design hinges
> on the test-flag to make complex configuration constraints discoverable at
> run-time. I think we need a few tools that random-walk the configuration
> space and check whether any configuration that returns -EINVAL in the test
> mode actually fails (and vice-versa), maybe excluding any other failures
> in the real modeset like -EIO (e.g. due to some stupid dp link failing to
> properly lock onto the stream).
>
> Cheers, Daniel
> --
> Daniel Vetter
> Mail: daniel at ffwll.ch
> Mobile: +41 (0)79 365 57 48