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, ...).
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.
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